From a9c743e2eb270e59c0dcb9069dc9706ee3013e6c Mon Sep 17 00:00:00 2001 From: Maxim Hayes Date: Fri, 15 Nov 2024 11:06:53 -0500 Subject: [PATCH] test(notifications): fix and improve - Fix async tests broken by recent commits. - Add setContext and telemetry verification --- packages/core/src/notifications/controller.ts | 6 ++-- packages/core/src/notifications/panelNode.ts | 27 +++++++++--------- .../src/test/notifications/controller.test.ts | 28 ++++++++++++++----- .../src/test/notifications/rendering.test.ts | 23 ++++++++------- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/packages/core/src/notifications/controller.ts b/packages/core/src/notifications/controller.ts index 9d1636c7a24..735721ad953 100644 --- a/packages/core/src/notifications/controller.ts +++ b/packages/core/src/notifications/controller.ts @@ -40,8 +40,6 @@ import { telemetry } from '../shared/telemetry/telemetry' export class NotificationsController { public static readonly suggestedPollIntervalMs = 1000 * 60 * 10 // 10 minutes - public readonly storageKey: globalKey - /** Internal memory state that is written to global state upon modification. */ private readonly state: NotificationsState @@ -49,7 +47,8 @@ export class NotificationsController { constructor( private readonly notificationsNode: NotificationsNode, - private readonly fetcher: NotificationFetcher = new RemoteFetcher() + private readonly fetcher: NotificationFetcher = new RemoteFetcher(), + public readonly storageKey: globalKey = 'aws.notifications' ) { if (!NotificationsController.#instance) { // Register on first creation only. @@ -57,7 +56,6 @@ export class NotificationsController { } NotificationsController.#instance = this - this.storageKey = 'aws.notifications' this.state = globals.globalState.tryGet(this.storageKey, NotificationsStateConstructor, { startUp: {}, emergency: {}, diff --git a/packages/core/src/notifications/panelNode.ts b/packages/core/src/notifications/panelNode.ts index cfcc0ad5eac..d3e93109da9 100644 --- a/packages/core/src/notifications/panelNode.ts +++ b/packages/core/src/notifications/panelNode.ts @@ -78,24 +78,23 @@ export class NotificationsNode implements TreeNode { } public refresh(): void { - if (!this.view) { - throw new ToolkitError('NotificationsNode: TreeView accessed without being registered.') - } - const totalNotifications = this.notificationCount() - if (totalNotifications > 0) { - this.view.badge = { - tooltip: `${totalNotifications} notification${totalNotifications > 1 ? 's' : ''}`, - value: totalNotifications, + if (this.view) { + if (totalNotifications > 0) { + this.view.badge = { + tooltip: `${totalNotifications} notification${totalNotifications > 1 ? 's' : ''}`, + value: totalNotifications, + } + this.view.title = `${NotificationsNode.title} (${totalNotifications})` + } else { + this.view.badge = undefined + this.view.title = NotificationsNode.title } - this.view.title = `${NotificationsNode.title} (${totalNotifications})` - void setContext(this.showContextStr, true) } else { - this.view.badge = undefined - this.view.title = NotificationsNode.title - void setContext(this.showContextStr, false) + logger.warn('NotificationsNode was refreshed but the view was not initialized!') } + void setContext(this.showContextStr, totalNotifications > 0) this.provider?.refresh() } @@ -240,7 +239,7 @@ export class NotificationsNode implements TreeNode { case 'updateAndReload': // Give things time to finish executing. globals.clock.setTimeout(() => { - void this.updateAndReload(notification.displayIf.extensionId) + return this.updateAndReload(notification.displayIf.extensionId) }, 1000) break case 'openUrl': diff --git a/packages/core/src/test/notifications/controller.test.ts b/packages/core/src/test/notifications/controller.test.ts index 669700a9cde..a1166f6b94f 100644 --- a/packages/core/src/test/notifications/controller.test.ts +++ b/packages/core/src/test/notifications/controller.test.ts @@ -9,6 +9,7 @@ import assert from 'assert' import sinon from 'sinon' import globals from '../../shared/extensionGlobals' import { randomUUID } from '../../shared' +import * as setContext from '../../shared/vscode/setContext' import { assertTelemetry, installFakeClock } from '../testUtil' import { NotificationFetcher, @@ -45,8 +46,9 @@ describe('Notifications Controller', function () { let controller: NotificationsController let fetcher: TestFetcher - let ruleEngineSpy: sinon.SinonSpy - let focusPanelSpy: sinon.SinonSpy + const ruleEngineSpy = sinon.spy(ruleEngine, 'shouldDisplayNotification') + const focusPanelSpy = sinon.spy(panelNode, 'focusPanel') + const setContextSpy = sinon.spy(setContext, 'setContext') function dismissNotification(notification: ToolkitNotification) { // We could call `controller.dismissNotification()`, but this emulates a call @@ -89,10 +91,7 @@ describe('Notifications Controller', function () { beforeEach(async function () { panelNode.setNotifications([], []) fetcher = new TestFetcher() - controller = new NotificationsController(panelNode, fetcher) - - ruleEngineSpy = sinon.spy(ruleEngine, 'shouldDisplayNotification') - focusPanelSpy = sinon.spy(panelNode, 'focusPanel') + controller = new NotificationsController(panelNode, fetcher, '_aws.test.notification' as any) await globals.globalState.update(controller.storageKey, { startUp: {} as NotificationData, @@ -102,8 +101,14 @@ describe('Notifications Controller', function () { }) afterEach(function () { - ruleEngineSpy.restore() + ruleEngineSpy.resetHistory() + focusPanelSpy.resetHistory() + setContextSpy.resetHistory() + }) + + after(function () { focusPanelSpy.restore() + setContextSpy.restore() }) it('can fetch and store startup notifications', async function () { @@ -135,6 +140,7 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.startUpNotifications, [content.notifications[0]]) assert.equal(panelNode.getChildren().length, 1) assert.equal(focusPanelSpy.callCount, 0) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) }) it('can fetch and store emergency notifications', async function () { @@ -166,6 +172,7 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.emergencyNotifications, [content.notifications[0]]) assert.equal(panelNode.getChildren().length, 1) assert.equal(focusPanelSpy.callCount, 1) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) }) it('can fetch and store both startup and emergency notifications', async function () { @@ -224,6 +231,7 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.emergencyNotifications, [emergencyContent.notifications[0]]) assert.equal(panelNode.getChildren().length, 2) assert.equal(focusPanelSpy.callCount, 1) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) }) it('dismisses a startup notification', async function () { @@ -241,6 +249,7 @@ describe('Notifications Controller', function () { assert.equal(panelNode.getChildren().length, 2) assert.equal(panelNode.startUpNotifications.length, 2) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) assert.deepStrictEqual(await globals.globalState.get(controller.storageKey), { startUp: { @@ -281,9 +290,11 @@ describe('Notifications Controller', function () { await controller.pollForStartUp(ruleEngine) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) await dismissNotification(content.notifications[0]) assert.equal(panelNode.getChildren().length, 0) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', false]) content.notifications.push(getValidTestNotification('id:startup2')) fetcher.setStartUpContent({ @@ -305,6 +316,7 @@ describe('Notifications Controller', function () { }) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) }) it('does not refocus emergency notifications', async function () { @@ -434,6 +446,7 @@ describe('Notifications Controller', function () { newlyReceived: [], }) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) fetcher.setEmergencyContent({ eTag: '1', @@ -455,6 +468,7 @@ describe('Notifications Controller', function () { }) assert.equal(panelNode.getChildren().length, 0) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', false]) }) it('does not rethrow errors when fetching', async function () { diff --git a/packages/core/src/test/notifications/rendering.test.ts b/packages/core/src/test/notifications/rendering.test.ts index 4df1caddc56..e446e53fdba 100644 --- a/packages/core/src/test/notifications/rendering.test.ts +++ b/packages/core/src/test/notifications/rendering.test.ts @@ -12,28 +12,20 @@ import { panelNode } from './controller.test' import { getTestWindow } from '../shared/vscode/window' import * as VsCodeUtils from '../../shared/utilities/vsCodeUtils' import { assertTextEditorContains, installFakeClock } from '../testUtil' +import { waitUntil } from '../../shared/utilities/timeoutUtils' +import globals from '../../shared/extensionGlobals' describe('Notifications Rendering', function () { let sandbox: sinon.SinonSandbox - let clock: FakeTimers.InstalledClock - - before(function () { - clock = installFakeClock() - }) beforeEach(function () { sandbox = sinon.createSandbox() }) afterEach(function () { - clock.reset() sandbox.restore() }) - after(function () { - clock.uninstall() - }) - // util to test txt pop-up under different senarios async function verifyTxtNotification(notification: ToolkitNotification) { const expectedContent = notification.uiRenderInstructions.content['en-US'].description @@ -103,14 +95,21 @@ describe('Notifications Rendering', function () { message.selectItem('Update and Reload') }) const excuteCommandStub = sandbox.stub(vscode.commands, 'executeCommand').resolves() + const telemetrySpy = sandbox.spy(globals.telemetry, 'flushRecords') const notification = getModalNotification() - await panelNode.openNotification(notification) // Update and Reload is put on a timer so that other methods (e.g. telemetry) can finish. - await clock.tickAsync(2000) + const clock: FakeTimers.InstalledClock = installFakeClock() + await panelNode.openNotification(notification) + + await clock.tickAsync(1000) + clock.uninstall() + + await waitUntil(async () => excuteCommandStub.called, { interval: 5, timeout: 5000 }) assert.ok(excuteCommandStub.calledWith('workbench.extensions.installExtension', 'aws.toolkit.fake.extension')) assert.ok(excuteCommandStub.calledWith('workbench.action.reloadWindow')) + assert.ok(telemetrySpy.calledOnce) }) it('executes openURL type button', async function () {