refacto: last cleanup of todos and some types

src/MongoMemoryServer.ts

@@ -1,7 +1,6 @@
 import { ChildProcess } from 'child_process';
 import tmp from 'tmp';
 import getPort from 'get-port';
-// import Debug from 'debug';  // TODO : Do we really need this package ?
 import { generateDbName } from './util/db_util';
 import MongoInstance from './util/MongoInstance';
 import { MongoBinaryOpts } from './util/MongoBinary';
@@ -39,7 +38,6 @@ export interface MongoInstanceDataT {
   replSet?: string;
 }
 
-// TODO: do we need to keep this function async ?
 const generateConnectionString = async (port: number, dbName: string): Promise<string> => {
   return `mongodb://127.0.0.1:${port}/${dbName}`;
 };
@@ -101,10 +99,6 @@ export default class MongoMemoryServer {
 
     const instOpts = this.opts.instance;
     data.port = await getPort({ port: (instOpts && instOpts.port) || undefined });
-    /*
-      this.debug = Debug(`Mongo[${data.port}]`); // TODO: Why do we dont just use this.debug here ?
-      this.debug.enabled = !!this.opts.debug; // Useful ?
-    */
     data.dbName = generateDbName(instOpts && instOpts.dbName);
     data.uri = await generateConnectionString(data.port, data.dbName);
     data.storageEngine = (instOpts && instOpts.storageEngine) || 'ephemeralForTest';
@@ -119,6 +113,7 @@ export default class MongoMemoryServer {
         unsafeCleanup: true,
       });
       data.dbPath = tmpDir.name;
+      data.tmpDir = tmpDir;
     }
 
     this.debug(`Starting MongoDB instance with following options: ${JSON.stringify(data)}`);
@@ -142,7 +137,6 @@ export default class MongoMemoryServer {
     });
     data.instance = instance;
     data.childProcess = instance.childProcess;
-    // data.tmpDir = tmpDir;
 
     return data;
   }

src/__tests__/replset-test.ts

@@ -33,10 +33,6 @@ describe('single server replset', () => {
     expect(dbName).toEqual('static');
   });
 
-  // TODO: This test provoke an unfinished async operation if MongoMemoryReplSet
-  // starts regardless of the autostart option
-  // Maybe should we re think how this functionality is tested by just mocking
-  // MongoMemoryReplSet.start function
   it('should not autostart if autostart: false', async () => {
     replSet = new MongoMemoryReplSet({ autoStart: false });
     await new Promise((resolve, reject) => {

src/util/MongoBinary.ts

@@ -115,7 +115,7 @@ export default class MongoBinary {
                 cwd: new RegExp(`node_modules${path.sep}mongodb-memory-server$`).test(process.cwd())
                   ? path.resolve(process.cwd(), '..', '..')
                   : process.cwd(),
-              }) || '', // TODO : path resolve doesnt accept string | null
+              }) || '',
               'mongodb-binaries'
             )),
       platform: process.env.MONGOMS_PLATFORM || os.platform(),

src/util/MongoBinaryDownload.ts

@@ -24,7 +24,7 @@ interface HttpDownloadOptions {
   port: string;
   path: string;
   method: 'GET' | 'POST';
-  agent: any | undefined; // TODO: fix type by adding declaration file for http-proxy module
+  agent: HttpsProxyAgent | undefined;
 }
 
 export default class MongoBinaryDownload {
@@ -106,9 +106,12 @@ export default class MongoBinaryDownload {
     return mongoDBArchive;
   }
 
-  async makeMD5check(urlForReferenceMD5: string, mongoDBArchive: string): Promise<boolean> {
+  async makeMD5check(
+    urlForReferenceMD5: string,
+    mongoDBArchive: string
+  ): Promise<boolean | undefined> {
     if (!this.checkMD5) {
-      return false; // TODO : why was it return undefined; ?
+      return undefined;
     }
     const mongoDBArchiveMd5 = await this.download(urlForReferenceMD5);
     const signatureContent = fs.readFileSync(mongoDBArchiveMd5).toString('UTF-8');
@@ -246,7 +249,6 @@ export default class MongoBinaryDownload {
   }
 
   printDownloadProgress(chunk: any): void {
-    // TODO : chunk type ?
     this.dlProgress.current += chunk.length;
 
     const now = Date.now();

src/util/__tests__/MongoBinary-test.ts

@@ -39,7 +39,7 @@ describe('MongoBinary', () => {
       expect(accessSpy).toHaveBeenCalledWith('/usr/local/bin/mongod', expect.any(Function));
 
       accessSpy.mockClear();
-      delete process.env.MONGOMS_SYSTEM_BINARY; // TODO : needed to add this because it was affecting the rest of the tests
+      delete process.env.MONGOMS_SYSTEM_BINARY;
     });
   });
 

src/util/__tests__/MongoBinaryDownload-test.ts

@@ -11,7 +11,6 @@ describe('MongoBinaryDownload', () => {
   });
 
   it('checkMD5 attribute can be set via constructor parameter', () => {
-    // TODO: need to add all these as MongoBinaryDownloadOpts becausing the type is only defined with mandatory fields
     expect(new MongoBinaryDownload({ checkMD5: true }).checkMD5).toBe(true);
     expect(new MongoBinaryDownload({ checkMD5: false }).checkMD5).toBe(false);
   });
@@ -93,6 +92,6 @@ the same as in the reference result`, () => {
     const du = new MongoBinaryDownload({});
     du.checkMD5 = false;
     const result = await du.makeMD5check('', '');
-    expect(result).toBe(false); // TODO: changed to false
+    expect(result).toBe(undefined);
   });
 });

src/util/__tests__/MongoInstance-test.ts

@@ -123,7 +123,7 @@ describe('MongodbInstance', () => {
       binary: { version: LATEST_VERSION },
     });
     const pid: any = mongod.getPid();
-    const killerPid: any = mongod.killerProcess && mongod.killerProcess.pid; // TODO: need to type this var but has a problem of null value in the class
+    const killerPid: any = mongod.killerProcess && mongod.killerProcess.pid;
     expect(pid).toBeGreaterThan(0);
     expect(killerPid).toBeGreaterThan(0);