From ed2947adfd6eb4c95904ad9f51433faa9e2dd1a9 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 8 Dec 2022 14:34:33 +0100 Subject: [PATCH 01/10] add ui tests for WalletAnalyticsNotificationBanner --- background/redux-slices/ui.ts | 2 +- ...WalletAnalyticsNotificationBanner.test.tsx | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 ui/components/Wallet/__tests__/WalletAnalyticsNotificationBanner.test.tsx diff --git a/background/redux-slices/ui.ts b/background/redux-slices/ui.ts index 31521dac70..ab7861268e 100644 --- a/background/redux-slices/ui.ts +++ b/background/redux-slices/ui.ts @@ -9,7 +9,7 @@ import { AccountSignerSettings } from "../ui" import { AccountState, addAddressNetwork } from "./accounts" import { createBackgroundAsyncThunk } from "./utils" -const defaultSettings = { +export const defaultSettings = { hideDust: false, defaultWallet: false, showTestNetworks: false, diff --git a/ui/components/Wallet/__tests__/WalletAnalyticsNotificationBanner.test.tsx b/ui/components/Wallet/__tests__/WalletAnalyticsNotificationBanner.test.tsx new file mode 100644 index 0000000000..27078d770f --- /dev/null +++ b/ui/components/Wallet/__tests__/WalletAnalyticsNotificationBanner.test.tsx @@ -0,0 +1,50 @@ +import React from "react" +import userEvent from "@testing-library/user-event" + +import { + defaultSettings, + initialState, +} from "@tallyho/tally-background/redux-slices/ui" + +import WalletAnalyticsNotificationBanner from "../WalletAnalyticsNotificationBanner" +import { renderWithProviders } from "../../../tests/test-utils" + +describe("WalletAnalyticsNotificationBanner", () => { + it("should be hidden by default", async () => { + const ui = renderWithProviders() + const bannerContainer = ui.container.firstChild + + expect(bannerContainer).toHaveClass("hide") + }) + it("should show when 'ui.settings.showAnalyticsNotification' is true", async () => { + const ui = renderWithProviders(, { + preloadedState: { + ui: { + ...initialState, + settings: { ...defaultSettings, showAnalyticsNotification: true }, + }, + }, + }) + const bannerContainer = ui.container.firstChild + + expect(bannerContainer).not.toHaveClass("hide") + }) + it("should disappear when the 'X' is clicked", async () => { + const ui = renderWithProviders(, { + preloadedState: { + ui: { + ...initialState, + settings: { ...defaultSettings, showAnalyticsNotification: true }, + }, + }, + }) + const bannerContainer = ui.container.firstChild + const closeButton = ui.getByLabelText("Close") + + expect(bannerContainer).not.toHaveClass("hide") + + await userEvent.click(closeButton) + + expect(bannerContainer).toHaveClass("hide") + }) +}) From 8126b2231c38da4cd066b08da0038dba4acc00c1 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Mon, 12 Dec 2022 14:43:37 +0100 Subject: [PATCH 02/10] initial analytics service integration tests --- __mocks__/uuid.ts | 9 ++ __mocks__/webextension-polyfill.ts | 16 +++ background/services/analytics/index.ts | 4 +- .../analytics/tests/index.integration.test.ts | 134 ++++++++++++++++++ background/tests/factories.ts | 13 ++ 5 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 __mocks__/uuid.ts create mode 100644 __mocks__/webextension-polyfill.ts create mode 100644 background/services/analytics/tests/index.integration.test.ts diff --git a/__mocks__/uuid.ts b/__mocks__/uuid.ts new file mode 100644 index 0000000000..9da5bfb782 --- /dev/null +++ b/__mocks__/uuid.ts @@ -0,0 +1,9 @@ +const mock = jest.createMockFromModule("uuid") +const actual = jest.requireActual("uuid") + +const v4Spy = jest.spyOn(actual, "v4") + +module.exports = { + ...mock, + v4: v4Spy, +} diff --git a/__mocks__/webextension-polyfill.ts b/__mocks__/webextension-polyfill.ts new file mode 100644 index 0000000000..78514c234e --- /dev/null +++ b/__mocks__/webextension-polyfill.ts @@ -0,0 +1,16 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import sinon from "sinon" + +const mock = jest.createMockFromModule( + "webextension-polyfill" +) + +const setUninstallURL = sinon.stub() + +module.exports = { + ...mock, + runtime: { + ...mock.runtime, + setUninstallURL, + }, +} diff --git a/background/services/analytics/index.ts b/background/services/analytics/index.ts index 6ae9448a8c..efd3a9cac2 100644 --- a/background/services/analytics/index.ts +++ b/background/services/analytics/index.ts @@ -65,8 +65,6 @@ export default class AnalyticsService extends BaseService { } if (isEnabled) { - this.sendAnalyticsEvent("Background start") - this.initializeListeners() const { uuid } = await this.getOrCreateAnalyticsUUID() @@ -74,6 +72,8 @@ export default class AnalyticsService extends BaseService { browser.runtime.setUninstallURL( `${process.env.WEBSITE_ORIGIN}/goodbye?uuid=${uuid}` ) + + await this.sendAnalyticsEvent("Background start") } } diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts new file mode 100644 index 0000000000..d894897ca5 --- /dev/null +++ b/background/services/analytics/tests/index.integration.test.ts @@ -0,0 +1,134 @@ +import sinon from "sinon" +import * as uuid from "uuid" + +import AnalyticsService from ".." +import * as features from "../../../features" +import { createAnalyticsService } from "../../../tests/factories" +import { Writeable } from "../../../types" +import PreferenceService from "../../preferences" +import { PreferenceDatabase } from "../../preferences/db" +import { AnalyticsDatabase } from "../db" + +type AnalyticsServiceExternalized = Omit & { + internalStartService: () => Promise + preferenceService: PreferenceService + db: AnalyticsDatabase +} + +type PreferenceServiceExternalized = Omit & { + db: PreferenceDatabase +} + +const sandbox = sinon.createSandbox() + +describe("AnalyticsService", () => { + let analyticsService: AnalyticsServiceExternalized + let preferenceService: PreferenceServiceExternalized + const runtimeFlagWritable = features.RuntimeFlag as Writeable< + typeof features.RuntimeFlag + > + beforeEach(async () => { + sandbox.restore() + jest.clearAllMocks() + + analyticsService = + (await createAnalyticsService()) as unknown as AnalyticsServiceExternalized + + preferenceService = + analyticsService.preferenceService as unknown as PreferenceServiceExternalized + + jest.spyOn(analyticsService.preferenceService, "getAnalyticsPreferences") + jest.spyOn(analyticsService.preferenceService, "updateAnalyticsPreferences") + jest.spyOn(analyticsService.preferenceService.emitter, "emit") + jest.spyOn(analyticsService.db, "setAnalyticsUUID") + + global.fetch = jest.fn() + }) + describe("the setup starts with the proper environment setup", () => { + it("PreferenceService should be initialized with isEnabled off and hasDefaultOnBeenTurnedOn off by default", async () => { + const { isEnabled, hasDefaultOnBeenTurnedOn } = + await preferenceService.getAnalyticsPreferences() + + expect(isEnabled).toBe(false) + expect(hasDefaultOnBeenTurnedOn).toBe(false) + expect(preferenceService.getAnalyticsPreferences).toBeCalled() + }) + it("should change the isEnabled output based on the changed feature flag", () => { + runtimeFlagWritable.SUPPORT_ANALYTICS = false + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = false + + expect(features.isEnabled(features.FeatureFlags.SUPPORT_ANALYTICS)).toBe( + false + ) + expect( + features.isEnabled(features.FeatureFlags.ENABLE_ANALYTICS_DEFAULT_ON) + ).toBe(false) + + runtimeFlagWritable.SUPPORT_ANALYTICS = true + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = true + + expect(features.isEnabled(features.FeatureFlags.SUPPORT_ANALYTICS)).toBe( + true + ) + expect( + features.isEnabled(features.FeatureFlags.ENABLE_ANALYTICS_DEFAULT_ON) + ).toBe(true) + }) + }) + describe("before the feature is released (the feature flags are off)", () => { + it("should not send any analytics events when both of the feature flags are off", async () => { + await analyticsService.startService() + await analyticsService.sendAnalyticsEvent("Background start") + + expect(fetch).not.toBeCalled() + }) + it("should not send any analytics events when only the support feature flag is on but the default on is not", async () => { + await analyticsService.startService() + await analyticsService.sendAnalyticsEvent("Background start") + + expect(fetch).not.toBeCalled() + }) + }) + describe("when the feature is released (feature flags are on, but settings is still off)", () => { + beforeEach(async () => { + runtimeFlagWritable.SUPPORT_ANALYTICS = true + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = true + + await analyticsService.startService() + }) + + it("should change isEnabled and hasDefaultOnBeenTurnedOn to true in PreferenceService", async () => { + // The default off value for analytics settings in PreferenceService has a test in the environment setup describe + expect(await preferenceService.getAnalyticsPreferences()).toStrictEqual({ + isEnabled: true, + hasDefaultOnBeenTurnedOn: true, + }) + expect(preferenceService.updateAnalyticsPreferences).toBeCalledTimes(1) + }) + it("should emit settings update event to notify UI", async () => { + expect(preferenceService.emitter.emit).toBeCalledTimes(1) + expect(preferenceService.emitter.emit).toBeCalledWith( + "updateAnalyticsPreferences", + { + isEnabled: true, + hasDefaultOnBeenTurnedOn: true, + } + ) + expect(preferenceService.updateAnalyticsPreferences).toBeCalledTimes(1) + }) + + it("should generate a new uuid", async () => { + expect(uuid.v4).toBeCalledTimes(1) + }) + + it.todo("should send 'New Install' and 'Background start' events") + }) + describe("feature is released and enabled", () => { + it.todo("should send 'Background start' event when the service starts") + it.todo("should set the uninstall url when the service starts") + }) + describe("feature is released but disabled", () => { + it.todo("should not send any event when the service starts") + it.todo("should not send any event when the service starts") + }) +}) diff --git a/background/tests/factories.ts b/background/tests/factories.ts index b425076664..5c9a5b7f45 100644 --- a/background/tests/factories.ts +++ b/background/tests/factories.ts @@ -12,6 +12,7 @@ import { BlockPrices, } from "../networks" import { + AnalyticsService, ChainService, IndexingService, KeyringService, @@ -84,6 +85,18 @@ type CreateSigningServiceOverrides = { chainService?: Promise } +export async function createAnalyticsService(overrides?: { + chainService?: Promise + preferenceService?: Promise +}): Promise { + const preferenceService = + overrides?.preferenceService ?? createPreferenceService() + return AnalyticsService.create( + overrides?.chainService ?? createChainService({ preferenceService }), + preferenceService + ) +} + export const createSigningService = async ( overrides: CreateSigningServiceOverrides = {} ): Promise => { From 0c93e2569d97d05f8c4c0d61dd2564f20f321f5c Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Wed, 14 Dec 2022 19:43:41 +0100 Subject: [PATCH 03/10] refactor tests for better readability Using the class["private"] notation is type safe and we can skip a lot of variables --- .../analytics/tests/index.integration.test.ts | 70 +++++++++---------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts index d894897ca5..dc703e708b 100644 --- a/background/services/analytics/tests/index.integration.test.ts +++ b/background/services/analytics/tests/index.integration.test.ts @@ -1,4 +1,9 @@ -import sinon from "sinon" +// We use the classInstance["privateMethodOrVariableName"] to access private properties in a type safe way +// without redefining the types. This is a typescript shortcoming that we can't easily redefine class member visibility. +// https://github.com/microsoft/TypeScript/issues/22677 +// POC https://www.typescriptlang.org/play?#code/MYGwhgzhAEBiD29oG8BQ0PWPAdhALgE4Cuw+8hAFAA6ECWAbmPgKbRgBc0B9OA5gBpotRszYAjLjmIBbcS0IBKFAF9Ua1CBb5oAM0TQAvNBwsA7nESUARGHHBrQgIwAmAMyLU2PPC0A6EHg+Sn14AG1bawBdZQB6WOgAeQBpVHisXAhfFgCgkMQIgH0waLiEgFEAJUrEyrSE0IiSqKNoAFZodKqayqA +/* eslint-disable @typescript-eslint/dot-notation */ + import * as uuid from "uuid" import AnalyticsService from ".." @@ -6,51 +11,37 @@ import * as features from "../../../features" import { createAnalyticsService } from "../../../tests/factories" import { Writeable } from "../../../types" import PreferenceService from "../../preferences" -import { PreferenceDatabase } from "../../preferences/db" -import { AnalyticsDatabase } from "../db" - -type AnalyticsServiceExternalized = Omit & { - internalStartService: () => Promise - preferenceService: PreferenceService - db: AnalyticsDatabase -} - -type PreferenceServiceExternalized = Omit & { - db: PreferenceDatabase -} - -const sandbox = sinon.createSandbox() describe("AnalyticsService", () => { - let analyticsService: AnalyticsServiceExternalized - let preferenceService: PreferenceServiceExternalized + let analyticsService: AnalyticsService + let preferenceService: PreferenceService const runtimeFlagWritable = features.RuntimeFlag as Writeable< typeof features.RuntimeFlag > + beforeAll(() => { + global.fetch = jest.fn() + // We need this set otherwise the posthog lib won't send the events + process.env.POSTHOG_API_KEY = "hey hey hey" + }) beforeEach(async () => { - sandbox.restore() jest.clearAllMocks() - analyticsService = - (await createAnalyticsService()) as unknown as AnalyticsServiceExternalized - - preferenceService = - analyticsService.preferenceService as unknown as PreferenceServiceExternalized + analyticsService = await createAnalyticsService() - jest.spyOn(analyticsService.preferenceService, "getAnalyticsPreferences") - jest.spyOn(analyticsService.preferenceService, "updateAnalyticsPreferences") - jest.spyOn(analyticsService.preferenceService.emitter, "emit") - jest.spyOn(analyticsService.db, "setAnalyticsUUID") + preferenceService = analyticsService["preferenceService"] - global.fetch = jest.fn() + jest.spyOn(preferenceService, "getAnalyticsPreferences") + jest.spyOn(preferenceService, "updateAnalyticsPreferences") + jest.spyOn(preferenceService.emitter, "emit") + jest.spyOn(analyticsService["db"], "setAnalyticsUUID") }) describe("the setup starts with the proper environment setup", () => { it("PreferenceService should be initialized with isEnabled off and hasDefaultOnBeenTurnedOn off by default", async () => { - const { isEnabled, hasDefaultOnBeenTurnedOn } = - await preferenceService.getAnalyticsPreferences() + expect(await preferenceService.getAnalyticsPreferences()).toStrictEqual({ + isEnabled: false, + hasDefaultOnBeenTurnedOn: false, + }) - expect(isEnabled).toBe(false) - expect(hasDefaultOnBeenTurnedOn).toBe(false) expect(preferenceService.getAnalyticsPreferences).toBeCalled() }) it("should change the isEnabled output based on the changed feature flag", () => { @@ -76,14 +67,18 @@ describe("AnalyticsService", () => { }) }) describe("before the feature is released (the feature flags are off)", () => { - it("should not send any analytics events when both of the feature flags are off", async () => { + beforeEach(async () => { + runtimeFlagWritable.SUPPORT_ANALYTICS = false + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = false + await analyticsService.startService() + }) + it("should not send any analytics events when both of the feature flags are off", async () => { await analyticsService.sendAnalyticsEvent("Background start") expect(fetch).not.toBeCalled() }) it("should not send any analytics events when only the support feature flag is on but the default on is not", async () => { - await analyticsService.startService() await analyticsService.sendAnalyticsEvent("Background start") expect(fetch).not.toBeCalled() @@ -118,7 +113,12 @@ describe("AnalyticsService", () => { }) it("should generate a new uuid", async () => { - expect(uuid.v4).toBeCalledTimes(1) + // Called once for generating the new user uuid + // and another time when sending the pothog event + expect(uuid.v4).toBeCalledTimes(2) + + // Posthog events are sent through global.fetch method + expect(fetch).toBeCalledTimes(1) }) it.todo("should send 'New Install' and 'Background start' events") From 9607020fd440650ab162ac8865878e12e3189872 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Wed, 14 Dec 2022 19:44:22 +0100 Subject: [PATCH 04/10] refactor lib so the env variables are changeable --- background/lib/posthog.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/background/lib/posthog.ts b/background/lib/posthog.ts index 2ca9cdcb38..ec90e2ce48 100644 --- a/background/lib/posthog.ts +++ b/background/lib/posthog.ts @@ -6,19 +6,13 @@ import logger from "./logger" export const POSTHOG_URL = process.env.POSTHOG_URL ?? "https://app.posthog.com/capture/" -// Destructuring doesn't work with env variables. process.nev is `MISSING ENV VAR` in that case -// eslint-disable-next-line prefer-destructuring -export const POSTHOG_API_KEY = process.env.POSTHOG_API_KEY - // Destructuring doesn't work with env variables. process.nev is `MISSING ENV VAR` in that case // eslint-disable-next-line prefer-destructuring export const USE_ANALYTICS_SOURCE = process.env.USE_ANALYTICS_SOURCE export function shouldSendPosthogEvents(): boolean { return ( - isEnabled(FeatureFlags.SUPPORT_ANALYTICS) && - !!POSTHOG_URL && - !!POSTHOG_API_KEY + isEnabled(FeatureFlags.SUPPORT_ANALYTICS) && !!process.env.POSTHOG_API_KEY ) } @@ -34,7 +28,7 @@ export function createPosthogPayload( // The unique or anonymous id of the user that triggered the event. distinct_id: personUUID, // api key - api_key: POSTHOG_API_KEY, + api_key: process.env.POSTHOG_API_KEY, // name of the event event: eventName, // Let's include a timestamp just to be sure. Optional. From 32ca0021bb14555c2f76a34a91f56f599641e3bb Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Wed, 14 Dec 2022 20:21:17 +0100 Subject: [PATCH 05/10] check initial events to be sent --- background/services/analytics/index.ts | 4 ++-- .../analytics/tests/index.integration.test.ts | 21 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/background/services/analytics/index.ts b/background/services/analytics/index.ts index efd3a9cac2..301f6a0a8d 100644 --- a/background/services/analytics/index.ts +++ b/background/services/analytics/index.ts @@ -67,13 +67,13 @@ export default class AnalyticsService extends BaseService { if (isEnabled) { this.initializeListeners() + await this.sendAnalyticsEvent("Background start") + const { uuid } = await this.getOrCreateAnalyticsUUID() browser.runtime.setUninstallURL( `${process.env.WEBSITE_ORIGIN}/goodbye?uuid=${uuid}` ) - - await this.sendAnalyticsEvent("Background start") } } diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts index dc703e708b..ce4a5b5dbe 100644 --- a/background/services/analytics/tests/index.integration.test.ts +++ b/background/services/analytics/tests/index.integration.test.ts @@ -11,6 +11,7 @@ import * as features from "../../../features" import { createAnalyticsService } from "../../../tests/factories" import { Writeable } from "../../../types" import PreferenceService from "../../preferences" +import * as posthog from "../../../lib/posthog" describe("AnalyticsService", () => { let analyticsService: AnalyticsService @@ -34,6 +35,8 @@ describe("AnalyticsService", () => { jest.spyOn(preferenceService, "updateAnalyticsPreferences") jest.spyOn(preferenceService.emitter, "emit") jest.spyOn(analyticsService["db"], "setAnalyticsUUID") + jest.spyOn(analyticsService, "sendAnalyticsEvent") + jest.spyOn(posthog, "sendPosthogEvent") }) describe("the setup starts with the proper environment setup", () => { it("PreferenceService should be initialized with isEnabled off and hasDefaultOnBeenTurnedOn off by default", async () => { @@ -114,14 +117,22 @@ describe("AnalyticsService", () => { it("should generate a new uuid", async () => { // Called once for generating the new user uuid - // and another time when sending the pothog event - expect(uuid.v4).toBeCalledTimes(2) + // and once for the 'Background start' and once for the `New install' event + expect(uuid.v4).toBeCalledTimes(3) + }) + it("should send 'New Install' and 'Background start' events", () => { // Posthog events are sent through global.fetch method - expect(fetch).toBeCalledTimes(1) - }) + // During initialization we send 2 events + expect(fetch).toBeCalledTimes(2) - it.todo("should send 'New Install' and 'Background start' events") + const [[, firstEventName], [, secondEventName]] = ( + posthog.sendPosthogEvent as unknown as jest.SpyInstance + ).mock.calls + + expect(firstEventName).toBe("New install") + expect(secondEventName).toBe("Background start") + }) }) describe("feature is released and enabled", () => { it.todo("should send 'Background start' event when the service starts") From da302e8d4ba679f85f0f60c97b76a8e23565f604 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 15 Dec 2022 11:53:31 +0100 Subject: [PATCH 06/10] move feature flag change reaction to service --- background/main.ts | 17 +++-------------- background/services/analytics/index.ts | 4 +++- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/background/main.ts b/background/main.ts index e9e4e124ab..41f87ccae0 100644 --- a/background/main.ts +++ b/background/main.ts @@ -1484,21 +1484,10 @@ export default class Main extends BaseService { } async connectAnalyticsService(): Promise { - const { hasDefaultOnBeenTurnedOn } = - await this.preferenceService.getAnalyticsPreferences() - - if ( - isEnabled(FeatureFlags.ENABLE_ANALYTICS_DEFAULT_ON) && - !hasDefaultOnBeenTurnedOn - ) { - // TODO: Remove this in the next release after we switch on - // analytics by default - await this.preferenceService.updateAnalyticsPreferences({ - isEnabled: true, - hasDefaultOnBeenTurnedOn: true, - }) + this.analyticsService.emitter.on("enableDefaultOn", () => { this.store.dispatch(setShowAnalyticsNotification(true)) - } + }) + this.preferenceService.emitter.on( "updateAnalyticsPreferences", async (analyticsPreferences: AnalyticsPreferences) => { diff --git a/background/services/analytics/index.ts b/background/services/analytics/index.ts index 301f6a0a8d..6854dc922b 100644 --- a/background/services/analytics/index.ts +++ b/background/services/analytics/index.ts @@ -11,7 +11,7 @@ import PreferenceService from "../preferences" import { FeatureFlags, isEnabled as isFeatureFlagEnabled } from "../../features" interface Events extends ServiceLifecycleEvents { - placeHolderEventForTypingPurposes: string + enableDefaultOn: void } /* @@ -62,6 +62,8 @@ export default class AnalyticsService extends BaseService { isEnabled, hasDefaultOnBeenTurnedOn, }) + + await this.emitter.emit("enableDefaultOn", undefined) } if (isEnabled) { From d9cb7da16ac8f589a5368381fc6f23dd821029a9 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 15 Dec 2022 11:54:52 +0100 Subject: [PATCH 07/10] refactor tests for robustness --- .../analytics/tests/index.integration.test.ts | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts index ce4a5b5dbe..20d8376ea8 100644 --- a/background/services/analytics/tests/index.integration.test.ts +++ b/background/services/analytics/tests/index.integration.test.ts @@ -28,19 +28,13 @@ describe("AnalyticsService", () => { jest.clearAllMocks() analyticsService = await createAnalyticsService() - preferenceService = analyticsService["preferenceService"] jest.spyOn(preferenceService, "getAnalyticsPreferences") - jest.spyOn(preferenceService, "updateAnalyticsPreferences") - jest.spyOn(preferenceService.emitter, "emit") - jest.spyOn(analyticsService["db"], "setAnalyticsUUID") - jest.spyOn(analyticsService, "sendAnalyticsEvent") - jest.spyOn(posthog, "sendPosthogEvent") }) describe("the setup starts with the proper environment setup", () => { it("PreferenceService should be initialized with isEnabled off and hasDefaultOnBeenTurnedOn off by default", async () => { - expect(await preferenceService.getAnalyticsPreferences()).toStrictEqual({ + expect(await preferenceService.getAnalyticsPreferences()).toEqual({ isEnabled: false, hasDefaultOnBeenTurnedOn: false, }) @@ -71,6 +65,8 @@ describe("AnalyticsService", () => { }) describe("before the feature is released (the feature flags are off)", () => { beforeEach(async () => { + jest.spyOn(analyticsService, "sendAnalyticsEvent") + runtimeFlagWritable.SUPPORT_ANALYTICS = false runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = false @@ -89,6 +85,12 @@ describe("AnalyticsService", () => { }) describe("when the feature is released (feature flags are on, but settings is still off)", () => { beforeEach(async () => { + jest.spyOn(analyticsService["db"], "setAnalyticsUUID") + jest.spyOn(analyticsService.emitter, "emit") + jest.spyOn(preferenceService, "updateAnalyticsPreferences") + jest.spyOn(preferenceService.emitter, "emit") + jest.spyOn(posthog, "sendPosthogEvent") + runtimeFlagWritable.SUPPORT_ANALYTICS = true runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = true @@ -97,13 +99,23 @@ describe("AnalyticsService", () => { it("should change isEnabled and hasDefaultOnBeenTurnedOn to true in PreferenceService", async () => { // The default off value for analytics settings in PreferenceService has a test in the environment setup describe - expect(await preferenceService.getAnalyticsPreferences()).toStrictEqual({ + expect(await preferenceService.getAnalyticsPreferences()).toEqual({ isEnabled: true, hasDefaultOnBeenTurnedOn: true, }) expect(preferenceService.updateAnalyticsPreferences).toBeCalledTimes(1) }) - it("should emit settings update event to notify UI", async () => { + it("should emit enableDefaultOn and settings update event to notify UI", async () => { + expect(analyticsService.emitter.emit).toBeCalledTimes(2) + expect(analyticsService.emitter.emit).toHaveBeenCalledWith( + "enableDefaultOn", + undefined + ) + expect(analyticsService.emitter.emit).toHaveBeenCalledWith( + "serviceStarted", + undefined + ) + expect(preferenceService.emitter.emit).toBeCalledTimes(1) expect(preferenceService.emitter.emit).toBeCalledWith( "updateAnalyticsPreferences", @@ -115,10 +127,12 @@ describe("AnalyticsService", () => { expect(preferenceService.updateAnalyticsPreferences).toBeCalledTimes(1) }) - it("should generate a new uuid", async () => { + it("should generate a new uuid and save it to database", async () => { // Called once for generating the new user uuid // and once for the 'Background start' and once for the `New install' event expect(uuid.v4).toBeCalledTimes(3) + + expect(analyticsService["db"].setAnalyticsUUID).toBeCalledTimes(1) }) it("should send 'New Install' and 'Background start' events", () => { @@ -126,12 +140,16 @@ describe("AnalyticsService", () => { // During initialization we send 2 events expect(fetch).toBeCalledTimes(2) - const [[, firstEventName], [, secondEventName]] = ( - posthog.sendPosthogEvent as unknown as jest.SpyInstance - ).mock.calls - - expect(firstEventName).toBe("New install") - expect(secondEventName).toBe("Background start") + expect(posthog.sendPosthogEvent).toHaveBeenCalledWith( + expect.anything(), + "New install", + undefined + ) + expect(posthog.sendPosthogEvent).toHaveBeenCalledWith( + expect.anything(), + "Background start", + undefined + ) }) }) describe("feature is released and enabled", () => { From 100a99361e508df2245d96602c282228c2a1a861 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 15 Dec 2022 19:17:20 +0100 Subject: [PATCH 08/10] add released and enabled tests --- __mocks__/uuid.ts | 8 +-- __mocks__/webextension-polyfill.ts | 15 +++--- background/services/analytics/index.ts | 16 +++--- .../analytics/tests/index.integration.test.ts | 54 +++++++++++++++++-- 4 files changed, 67 insertions(+), 26 deletions(-) diff --git a/__mocks__/uuid.ts b/__mocks__/uuid.ts index 9da5bfb782..e493a51ba2 100644 --- a/__mocks__/uuid.ts +++ b/__mocks__/uuid.ts @@ -1,9 +1,9 @@ -const mock = jest.createMockFromModule("uuid") -const actual = jest.requireActual("uuid") +const uuidMock = jest.createMockFromModule("uuid") +const uuidActual = jest.requireActual("uuid") -const v4Spy = jest.spyOn(actual, "v4") +const v4Spy = jest.spyOn(uuidActual, "v4") module.exports = { - ...mock, + ...uuidMock, v4: v4Spy, } diff --git a/__mocks__/webextension-polyfill.ts b/__mocks__/webextension-polyfill.ts index 78514c234e..f080317259 100644 --- a/__mocks__/webextension-polyfill.ts +++ b/__mocks__/webextension-polyfill.ts @@ -1,16 +1,13 @@ -// eslint-disable-next-line import/no-extraneous-dependencies -import sinon from "sinon" +const browserMock = jest.createMockFromModule< + typeof import("webextension-polyfill") +>("webextension-polyfill") -const mock = jest.createMockFromModule( - "webextension-polyfill" -) - -const setUninstallURL = sinon.stub() +const setUninstallURL = jest.fn() module.exports = { - ...mock, + ...browserMock, runtime: { - ...mock.runtime, + ...browserMock.runtime, setUninstallURL, }, } diff --git a/background/services/analytics/index.ts b/background/services/analytics/index.ts index 6854dc922b..2e2c162935 100644 --- a/background/services/analytics/index.ts +++ b/background/services/analytics/index.ts @@ -69,13 +69,17 @@ export default class AnalyticsService extends BaseService { if (isEnabled) { this.initializeListeners() - await this.sendAnalyticsEvent("Background start") - - const { uuid } = await this.getOrCreateAnalyticsUUID() + const { uuid, isNew } = await this.getOrCreateAnalyticsUUID() browser.runtime.setUninstallURL( `${process.env.WEBSITE_ORIGIN}/goodbye?uuid=${uuid}` ) + + if (isNew) { + await this.sendAnalyticsEvent("New install") + } + + await this.sendAnalyticsEvent("Background start") } } @@ -93,11 +97,7 @@ export default class AnalyticsService extends BaseService { const { isEnabled } = await this.preferenceService.getAnalyticsPreferences() if (isEnabled) { - const { uuid, isNew } = await this.getOrCreateAnalyticsUUID() - - if (isNew) { - sendPosthogEvent(uuid, "New install", payload) - } + const { uuid } = await this.getOrCreateAnalyticsUUID() sendPosthogEvent(uuid, eventName, payload) } diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts index 20d8376ea8..dff3fe0fae 100644 --- a/background/services/analytics/tests/index.integration.test.ts +++ b/background/services/analytics/tests/index.integration.test.ts @@ -5,6 +5,7 @@ /* eslint-disable @typescript-eslint/dot-notation */ import * as uuid from "uuid" +import browser from "webextension-polyfill" import AnalyticsService from ".." import * as features from "../../../features" @@ -29,11 +30,11 @@ describe("AnalyticsService", () => { analyticsService = await createAnalyticsService() preferenceService = analyticsService["preferenceService"] - - jest.spyOn(preferenceService, "getAnalyticsPreferences") }) describe("the setup starts with the proper environment setup", () => { it("PreferenceService should be initialized with isEnabled off and hasDefaultOnBeenTurnedOn off by default", async () => { + jest.spyOn(preferenceService, "getAnalyticsPreferences") + expect(await preferenceService.getAnalyticsPreferences()).toEqual({ isEnabled: false, hasDefaultOnBeenTurnedOn: false, @@ -152,9 +153,52 @@ describe("AnalyticsService", () => { ) }) }) - describe("feature is released and enabled", () => { - it.todo("should send 'Background start' event when the service starts") - it.todo("should set the uninstall url when the service starts") + describe("feature is released and enabled (analytics uuid has been created earlier)", () => { + beforeEach(async () => { + jest.spyOn(analyticsService, "sendAnalyticsEvent") + jest.spyOn(preferenceService, "updateAnalyticsPreferences") + jest + .spyOn(preferenceService, "getAnalyticsPreferences") + .mockImplementation(() => + Promise.resolve({ + isEnabled: true, + hasDefaultOnBeenTurnedOn: true, + }) + ) + + runtimeFlagWritable.SUPPORT_ANALYTICS = true + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = true + + // Initialize analytics uuid + await analyticsService["getOrCreateAnalyticsUUID"]() + + await analyticsService.startService() + }) + it("should not run the initialization flow", async () => { + // uuid should be already present + expect(await analyticsService["getOrCreateAnalyticsUUID"]()).toEqual( + expect.objectContaining({ isNew: false }) + ) + + expect(analyticsService.sendAnalyticsEvent).not.toHaveBeenCalledWith( + expect.anything(), + "New install", + undefined + ) + + expect( + preferenceService.updateAnalyticsPreferences + ).not.toHaveBeenCalled() + }) + it("should send 'Background start' event when the service starts", async () => { + expect(analyticsService.sendAnalyticsEvent).toBeCalledTimes(1) + expect(analyticsService.sendAnalyticsEvent).toBeCalledWith( + "Background start" + ) + }) + it("should set the uninstall url when the service starts", async () => { + expect(browser.runtime.setUninstallURL).toBeCalledTimes(1) + }) }) describe("feature is released but disabled", () => { it.todo("should not send any event when the service starts") From 6bf3c1d7cca3f20d93139a50a0fdd4d0ac68e499 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 15 Dec 2022 19:28:18 +0100 Subject: [PATCH 09/10] add tests for released but disabled feature --- .../analytics/tests/index.integration.test.ts | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/background/services/analytics/tests/index.integration.test.ts b/background/services/analytics/tests/index.integration.test.ts index dff3fe0fae..a4b7bc28ea 100644 --- a/background/services/analytics/tests/index.integration.test.ts +++ b/background/services/analytics/tests/index.integration.test.ts @@ -201,7 +201,38 @@ describe("AnalyticsService", () => { }) }) describe("feature is released but disabled", () => { - it.todo("should not send any event when the service starts") - it.todo("should not send any event when the service starts") + beforeEach(async () => { + jest.spyOn(analyticsService, "sendAnalyticsEvent") + jest.spyOn(posthog, "sendPosthogEvent") + jest + .spyOn(preferenceService, "getAnalyticsPreferences") + .mockImplementation(() => + Promise.resolve({ + isEnabled: false, + hasDefaultOnBeenTurnedOn: true, + }) + ) + + runtimeFlagWritable.SUPPORT_ANALYTICS = true + runtimeFlagWritable.ENABLE_ANALYTICS_DEFAULT_ON = true + + // Initialize analytics uuid + await analyticsService["getOrCreateAnalyticsUUID"]() + + await analyticsService.startService() + }) + it("should not send any event when the service starts", async () => { + expect(analyticsService.sendAnalyticsEvent).not.toBeCalled() + + expect(posthog.sendPosthogEvent).not.toBeCalled() + expect(fetch).not.toBeCalled() + }) + it("should not send any event when the 'sendAnalyticsEvent()' method is called", async () => { + await analyticsService.sendAnalyticsEvent("Background start") + expect(analyticsService.sendAnalyticsEvent).toBeCalledTimes(1) + + expect(posthog.sendPosthogEvent).not.toBeCalled() + expect(fetch).not.toBeCalled() + }) }) }) From abcbaa7d40d9b42a7d72050b1c8c849d0f36ef14 Mon Sep 17 00:00:00 2001 From: Gergo Nagy Date: Thu, 15 Dec 2022 20:12:11 +0100 Subject: [PATCH 10/10] unify webextension mocking with auto mock --- __mocks__/webextension-polyfill.ts | 26 +++++++++++++++++--- background/tests/keyring-integration.test.ts | 2 ++ setupJest.ts | 21 ---------------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/__mocks__/webextension-polyfill.ts b/__mocks__/webextension-polyfill.ts index f080317259..479ed2ee9c 100644 --- a/__mocks__/webextension-polyfill.ts +++ b/__mocks__/webextension-polyfill.ts @@ -1,13 +1,33 @@ +import { Tabs } from "webextension-polyfill" + const browserMock = jest.createMockFromModule< typeof import("webextension-polyfill") >("webextension-polyfill") -const setUninstallURL = jest.fn() - module.exports = { ...browserMock, + alarms: { + create: () => {}, + clear: () => {}, + onAlarm: { + addListener: () => {}, + removeListener: () => {}, + }, + }, + extension: { + ...browserMock.extension, + getBackgroundPage: jest.fn(), + }, + tabs: { + ...browserMock.tabs, + getCurrent: jest.fn(() => + // getCurrent can return undefined if there is no tab, and we act accordingly + // in the code. + Promise.resolve(undefined as unknown as Tabs.Tab) + ), + }, runtime: { ...browserMock.runtime, - setUninstallURL, + setUninstallURL: jest.fn(), }, } diff --git a/background/tests/keyring-integration.test.ts b/background/tests/keyring-integration.test.ts index ebce733d6f..d832bed11b 100644 --- a/background/tests/keyring-integration.test.ts +++ b/background/tests/keyring-integration.test.ts @@ -1,4 +1,6 @@ import { webcrypto } from "crypto" +import browser from "webextension-polyfill" + import KeyringService, { Keyring, MAX_KEYRING_IDLE_TIME, diff --git a/setupJest.ts b/setupJest.ts index 07f6f0ccc3..e47bde1951 100644 --- a/setupJest.ts +++ b/setupJest.ts @@ -1,6 +1,5 @@ import * as util from "util" import Dexie from "dexie" -import { Tabs } from "webextension-polyfill" // ref: https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom // ref: https://github.com/jsdom/jsdom/issues/2524 @@ -21,26 +20,6 @@ Object.defineProperty(window.navigator, "usb", { }, }) -Object.defineProperty(browser, "alarms", { - writable: true, - value: { - create: () => {}, - clear: () => {}, - onAlarm: { - addListener: () => {}, - removeListener: () => {}, - }, - }, -}) - -// Mock top-level logger calls. -browser.extension.getBackgroundPage = jest.fn() -browser.tabs.getCurrent = jest.fn(() => - // getCurrent can return undefined if there is no tab, and we act accordingly - // in the code. - Promise.resolve(undefined as unknown as Tabs.Tab) -) - // Prevent Dexie from caching indexedDB global so fake-indexeddb // can reset properly. Object.defineProperty(Dexie.dependencies, "indexedDB", {