Skip to content
This repository has been archived by the owner on Apr 3, 2024. It is now read-only.

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

Merged
merged 3 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/agent/debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
mctavish marked this conversation as resolved.
Show resolved Hide resolved
that.debuggeeRegistered.resolve();
if (!that.fetcherActive) {
that.startListeningForBreakpoints_();
}
Expand All @@ -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();
mctavish marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down
24 changes: 13 additions & 11 deletions test/test-debuglet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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', () => {
Expand All @@ -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},
Expand All @@ -1237,7 +1234,7 @@ describe('Debuglet', () => {
});

debuglet.start();
});*/
});

it('should reject breakpoints with conditions when allowExpressions=false', function (done) {
this.timeout(2000);
Expand Down Expand Up @@ -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
Expand All @@ -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}})
Expand All @@ -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();
Expand All @@ -1476,7 +1478,7 @@ describe('Debuglet', () => {
});

debuglet.start();
});*/
});
});

describe('map subtract', () => {
Expand Down
4 changes: 2 additions & 2 deletions test/test-firebase-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class MockReference {
if (onComplete) {
onComplete(null);
}
return new Promise<any>(resolve => resolve(null));
return Promise.resolve();
}

getOrAdd(key: string): MockReference {
Expand Down Expand Up @@ -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(
Expand Down