fix: Fix and re-enable previously broken tests (#1075)

mctavish · web-flow · commit dd61d35822da · 2022-05-11T09:57:17.000-04:00
* Fix and re-enable previously broken tests
diff --git a/src/agent/debuglet.ts b/src/agent/debuglet.ts
@@ -192,6 +192,8 @@ export class Debuglet extends EventEmitter {
   private controller: Controller | null;
   private completedBreakpointMap: {[key: string]: boolean};
 
+  // The following four variables are used for the "isReady" functionality.
+
   // breakpointFetchedTimestamp represents the last timestamp when
   // breakpointFetched was resolved, which means breakpoint update was
   // successful.
@@ -765,8 +767,8 @@ export class Debuglet extends EventEmitter {
             }
           ).debuggee.id;
           // TODO: Handle the case when `result` is undefined.
-          that.emit('registered', (result as {debuggee: Debuggee}).debuggee.id); // FIXME: Do we need this?
-          that.debuggeeRegistered.resolve(); // FIXME: Do we need this?
+          that.emit('registered', (result as {debuggee: Debuggee}).debuggee.id);
+          that.debuggeeRegistered.resolve();
           if (!that.fetcherActive) {
             that.startListeningForBreakpoints_();
           }
@@ -788,16 +790,23 @@ export class Debuglet extends EventEmitter {
             err.name === 'RegistrationExpiredError'
               ? 0
               : this.config.internal.registerDelayOnFetcherErrorSec;
+          // The debuglet is no longer ready and the promises are stale.
+          this.updatePromise();
           this.scheduleRegistration_(delay);
         }
 
+        this.breakpointFetchedTimestamp = Date.now();
+        if (this.breakpointFetched) {
+          this.breakpointFetched.resolve();
+          this.breakpointFetched = null;
+        }
         this.updateActiveBreakpoints_(breakpoints);
       }
     );
   }
 
   /**
-   * updatePromise_ is called when debuggee is expired. debuggeeRegistered
+   * updatePromise is called when debuggee is expired. debuggeeRegistered
    * CachedPromise will be refreshed. Also, breakpointFetched CachedPromise will
    * be resolved so that uses (such as GCF users) will not hang forever to wait
    * non-fetchable breakpoints.
diff --git a/test/test-debuglet.ts b/test/test-debuglet.ts
@@ -1169,8 +1169,7 @@ describe('Debuglet', () => {
       debuglet.start();
     });
 
-    /*it('should have breakpoints fetched when promise is resolved', function (done) {
-      // FIXME: Broken.
+    it('should have breakpoints fetched when promise is resolved', function (done) {
       this.timeout(2000);
       const breakpoint: stackdriver.Breakpoint = {
         id: 'test1',
@@ -1189,7 +1188,6 @@ describe('Debuglet', () => {
         .post(REGISTER_PATH)
         .reply(200, {debuggee: {id: DEBUGGEE_ID}})
         .get(BPS_PATH + '?successOnTimeout=true')
-        .twice()
         .reply(200, {breakpoints: [breakpoint]});
       const debugPromise = debuglet.isReadyManager.isReady();
       debuglet.once('registered', () => {
@@ -1212,7 +1210,6 @@ describe('Debuglet', () => {
     });
 
     it('should resolve breakpointFetched promise when registration expires', function (done) {
-      // FIXME: Broken.
       this.timeout(2000);
       const debug = new Debug(
         {projectId: 'fake-project', credentials: fakeCredentials},
@@ -1237,7 +1234,7 @@ describe('Debuglet', () => {
       });
 
       debuglet.start();
-    });*/
+    });
 
     it('should reject breakpoints with conditions when allowExpressions=false', function (done) {
       this.timeout(2000);
@@ -1425,8 +1422,7 @@ describe('Debuglet', () => {
     // The test expires a breakpoint and then has the api respond with
     // the breakpoint listed as active. It validates that the breakpoint
     // is only expired with the server once.
-    /*it('should not update expired breakpoints', function (done) {
-      // FIXME: Broken
+    it('should not update expired breakpoints', function (done) {
       const debug = new Debug(
         {projectId: 'fake-project', credentials: fakeCredentials},
         packageInfo
@@ -1440,6 +1436,7 @@ describe('Debuglet', () => {
         forceNewAgent_: true,
       });
       const debuglet = new Debuglet(debug, config);
+      let unexpectedUpdate = false;
       const scope = nock(config.apiUrl)
         .post(REGISTER_PATH)
         .reply(200, {debuggee: {id: DEBUGGEE_ID}})
@@ -1459,15 +1456,20 @@ describe('Debuglet', () => {
         .times(4)
         .reply(200, {breakpoints: [bp]});
 
+      // Get ready to fail the test if any additional updates come through.
+      nock.emitter.on('no match', req => {
+        if (req.path.startsWith(BPS_PATH) && req.method === 'PUT') {
+          unexpectedUpdate = true;
+        }
+      });
+
       debuglet.once('registered', (id: string) => {
         assert.strictEqual(id, DEBUGGEE_ID);
         setTimeout(() => {
           assert.deepStrictEqual(debuglet.activeBreakpointMap.test, bp);
           setTimeout(() => {
             assert(!debuglet.activeBreakpointMap.test);
-            // Fetcher disables if we re-update since endpoint isn't mocked
-            // twice
-            assert(debuglet.fetcherActive);
+            assert(!unexpectedUpdate, 'unexpected update request');
             debuglet.stop();
             scope.done();
             done();
@@ -1476,7 +1478,7 @@ describe('Debuglet', () => {
       });
 
       debuglet.start();
-    });*/
+    });
   });
 
   describe('map subtract', () => {
diff --git a/test/test-firebase-controller.ts b/test/test-firebase-controller.ts
@@ -57,7 +57,7 @@ class MockReference {
     if (onComplete) {
       onComplete(null);
     }
-    return new Promise<any>(resolve => resolve(null));
+    return Promise.resolve();
   }
 
   getOrAdd(key: string): MockReference {
@@ -101,7 +101,7 @@ class MockReference {
     if (creating && this.parentRef) {
       this.parentRef.childAdded(this.key, value);
     }
-    return new Promise<any>(resolve => resolve(null));
+    return Promise.resolve();
   }
 
   on(

Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ class MockReference {`
`57`	`57`	`if (onComplete) {`
`58`	`58`	`onComplete(null);`
`59`	`59`	`}`
`60`		`- return new Promise<any>(resolve => resolve(null));`
	`60`	`+ return Promise.resolve();`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`getOrAdd(key: string): MockReference {`
`@@ -101,7 +101,7 @@ class MockReference {`
`101`	`101`	`if (creating && this.parentRef) {`
`102`	`102`	`this.parentRef.childAdded(this.key, value);`
`103`	`103`	`}`
`104`		`- return new Promise<any>(resolve => resolve(null));`
	`104`	`+ return Promise.resolve();`
`105`	`105`	`}`
`106`	`106`
`107`	`107`	`on(`