From 4d95c968d0073f4a3ee18ecc375451ab9a4cda44 Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Tue, 19 Nov 2024 09:49:49 +0100 Subject: [PATCH 1/8] Default feature flags as an object --- .../src/lib/constants/environment.constants.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/constants/environment.constants.ts b/frontend/src/lib/constants/environment.constants.ts index eaa853d045d..54f2f688772 100644 --- a/frontend/src/lib/constants/environment.constants.ts +++ b/frontend/src/lib/constants/environment.constants.ts @@ -19,6 +19,15 @@ export interface FeatureFlags { TEST_FLAG_EDITABLE: T; TEST_FLAG_NOT_EDITABLE: T; } +const defaultFeatureFlags: FeatureFlags = { + ENABLE_CKBTC: false, + ENABLE_CKTESTBTC: false, + DISABLE_IMPORT_TOKEN_VALIDATION_FOR_TESTING: false, + ENABLE_NEURON_VISIBILITY: false, + ENABLE_PERIODIC_FOLLOWING_CONFIRMATION: false, + TEST_FLAG_EDITABLE: false, + TEST_FLAG_NOT_EDITABLE: false, +}; export type FeatureKey = keyof FeatureFlags; @@ -27,10 +36,10 @@ export type FeatureKey = keyof FeatureFlags; * * @see feature-flags.store.ts to use feature flags */ -export const FEATURE_FLAG_ENVIRONMENT: FeatureFlags = JSON.parse( - envVars?.featureFlags ?? - '{"ENABLE_CKBTC": true, "ENABLE_CKTESTBTC": false, "ENABLE_SNS_TYPES_FILTER": false, "ENABLE_NEURON_VISIBILITY": false}' -); +export const FEATURE_FLAG_ENVIRONMENT: FeatureFlags = + envVars?.featureFlags + ? JSON.parse(envVars?.featureFlags) + : defaultFeatureFlags; export const IS_TESTNET: boolean = DFX_NETWORK !== "mainnet" && From cd62d2318f46108ae6850d3f135422d7178b4a3c Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 12:00:14 +0100 Subject: [PATCH 2/8] getFeatureFlagsFromEnv --- .../src/lib/constants/environment.constants.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/frontend/src/lib/constants/environment.constants.ts b/frontend/src/lib/constants/environment.constants.ts index 54f2f688772..4c0c25c642c 100644 --- a/frontend/src/lib/constants/environment.constants.ts +++ b/frontend/src/lib/constants/environment.constants.ts @@ -20,7 +20,7 @@ export interface FeatureFlags { TEST_FLAG_NOT_EDITABLE: T; } const defaultFeatureFlags: FeatureFlags = { - ENABLE_CKBTC: false, + ENABLE_CKBTC: true, ENABLE_CKTESTBTC: false, DISABLE_IMPORT_TOKEN_VALIDATION_FOR_TESTING: false, ENABLE_NEURON_VISIBILITY: false, @@ -31,15 +31,24 @@ const defaultFeatureFlags: FeatureFlags = { export type FeatureKey = keyof FeatureFlags; +const getFeatureFlagsFromEnv = (): FeatureFlags => { + let featureFlags = {}; + try { + featureFlags = JSON.parse(envVars?.featureFlags); + } finally { + // do nothing + } + // Complement the default flags with the ones from the environment to avoid missing flags. + return { ...defaultFeatureFlags, ...featureFlags }; +}; + /** * DO NOT USE DIRECTLY * * @see feature-flags.store.ts to use feature flags */ export const FEATURE_FLAG_ENVIRONMENT: FeatureFlags = - envVars?.featureFlags - ? JSON.parse(envVars?.featureFlags) - : defaultFeatureFlags; + getFeatureFlagsFromEnv(); export const IS_TESTNET: boolean = DFX_NETWORK !== "mainnet" && From 4b3a7b3788f067cb66e074da3d74fa0fc8018a16 Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 12:00:30 +0100 Subject: [PATCH 3/8] Test default feature flags --- .../constants/environment.constants.spec.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 frontend/src/tests/lib/constants/environment.constants.spec.ts diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts new file mode 100644 index 00000000000..2dfede26f00 --- /dev/null +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -0,0 +1,37 @@ +import * as envVarsUtils from "$lib/utils/env-vars.utils"; + +describe("FEATURE_FLAG_ENVIRONMENT", () => { + const environmentVars = envVarsUtils.getEnvVars(); + + beforeEach(() => { + vi.resetModules(); + }); + + it("should equal the environment values", async () => { + const { FEATURE_FLAG_ENVIRONMENT } = await import( + "$lib/constants/environment.constants" + ); + const expectedFlags = JSON.parse(environmentVars.featureFlags); + expect(FEATURE_FLAG_ENVIRONMENT).toEqual(expectedFlags); + }); + + it("should contain entries substituted with default values", async () => { + vi.spyOn(envVarsUtils, "getEnvVars").mockReturnValue({ + ...environmentVars, + featureFlags: JSON.stringify({}), + }); + + const { FEATURE_FLAG_ENVIRONMENT } = await import( + "$lib/constants/environment.constants" + ); + expect(FEATURE_FLAG_ENVIRONMENT).toEqual({ + ENABLE_CKBTC: true, + ENABLE_CKTESTBTC: false, + DISABLE_IMPORT_TOKEN_VALIDATION_FOR_TESTING: false, + ENABLE_NEURON_VISIBILITY: false, + ENABLE_PERIODIC_FOLLOWING_CONFIRMATION: false, + TEST_FLAG_EDITABLE: false, + TEST_FLAG_NOT_EDITABLE: false, + }); + }); +}); From fc4136ea8d3467220c9e19301671e9628255f3e2 Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 12:08:31 +0100 Subject: [PATCH 4/8] test: use defaultFeatureFlagValues directly --- frontend/src/lib/constants/environment.constants.ts | 4 ++-- .../tests/lib/constants/environment.constants.spec.ts | 11 ++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/frontend/src/lib/constants/environment.constants.ts b/frontend/src/lib/constants/environment.constants.ts index 777be1fc3fc..8caab62cc41 100644 --- a/frontend/src/lib/constants/environment.constants.ts +++ b/frontend/src/lib/constants/environment.constants.ts @@ -19,7 +19,7 @@ export interface FeatureFlags { TEST_FLAG_EDITABLE: T; TEST_FLAG_NOT_EDITABLE: T; } -const defaultFeatureFlags: FeatureFlags = { +export const defaultFeatureFlagValues: FeatureFlags = { ENABLE_CKBTC: true, ENABLE_CKTESTBTC: false, DISABLE_IMPORT_TOKEN_VALIDATION_FOR_TESTING: false, @@ -39,7 +39,7 @@ const getFeatureFlagsFromEnv = (): FeatureFlags => { // do nothing } // Complement the default flags with the ones from the environment to avoid missing flags. - return { ...defaultFeatureFlags, ...featureFlags }; + return { ...defaultFeatureFlagValues, ...featureFlags }; }; /** diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts index 2dfede26f00..0cf4faf28d1 100644 --- a/frontend/src/tests/lib/constants/environment.constants.spec.ts +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -1,3 +1,4 @@ +import { defaultFeatureFlagValues } from "$lib/constants/environment.constants"; import * as envVarsUtils from "$lib/utils/env-vars.utils"; describe("FEATURE_FLAG_ENVIRONMENT", () => { @@ -24,14 +25,6 @@ describe("FEATURE_FLAG_ENVIRONMENT", () => { const { FEATURE_FLAG_ENVIRONMENT } = await import( "$lib/constants/environment.constants" ); - expect(FEATURE_FLAG_ENVIRONMENT).toEqual({ - ENABLE_CKBTC: true, - ENABLE_CKTESTBTC: false, - DISABLE_IMPORT_TOKEN_VALIDATION_FOR_TESTING: false, - ENABLE_NEURON_VISIBILITY: false, - ENABLE_PERIODIC_FOLLOWING_CONFIRMATION: false, - TEST_FLAG_EDITABLE: false, - TEST_FLAG_NOT_EDITABLE: false, - }); + expect(FEATURE_FLAG_ENVIRONMENT).toEqual(defaultFeatureFlagValues); }); }); From cd84c6dc722193ea416ab1655cce65048b5b2af7 Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 12:09:38 +0100 Subject: [PATCH 5/8] Comment --- frontend/src/tests/lib/constants/environment.constants.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts index 0cf4faf28d1..9eb9ff70f3f 100644 --- a/frontend/src/tests/lib/constants/environment.constants.spec.ts +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -16,7 +16,7 @@ describe("FEATURE_FLAG_ENVIRONMENT", () => { expect(FEATURE_FLAG_ENVIRONMENT).toEqual(expectedFlags); }); - it("should contain entries substituted with default values", async () => { + it("should contain missing entries substituted with default values", async () => { vi.spyOn(envVarsUtils, "getEnvVars").mockReturnValue({ ...environmentVars, featureFlags: JSON.stringify({}), From 3915ab930c8eea92febb322a283773c47da7005d Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 14:46:04 +0100 Subject: [PATCH 6/8] Log error on invalid featureFlags env --- .../src/lib/constants/environment.constants.ts | 4 ++-- frontend/src/tests/lib/api/canisters.api.spec.ts | 1 + .../lib/constants/environment.constants.spec.ts | 16 ++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/frontend/src/lib/constants/environment.constants.ts b/frontend/src/lib/constants/environment.constants.ts index 8caab62cc41..1055ef98189 100644 --- a/frontend/src/lib/constants/environment.constants.ts +++ b/frontend/src/lib/constants/environment.constants.ts @@ -35,8 +35,8 @@ const getFeatureFlagsFromEnv = (): FeatureFlags => { let featureFlags = {}; try { featureFlags = JSON.parse(envVars?.featureFlags); - } finally { - // do nothing + } catch (e) { + console.error("Error parsing featureFlags", e); } // Complement the default flags with the ones from the environment to avoid missing flags. return { ...defaultFeatureFlagValues, ...featureFlags }; diff --git a/frontend/src/tests/lib/api/canisters.api.spec.ts b/frontend/src/tests/lib/api/canisters.api.spec.ts index 2931d7b253c..b8fd6183ef9 100644 --- a/frontend/src/tests/lib/api/canisters.api.spec.ts +++ b/frontend/src/tests/lib/api/canisters.api.spec.ts @@ -422,6 +422,7 @@ describe("canisters-api", () => { }); it("should not notify if transfer fails", async () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); mockLedgerCanister.transfer.mockRejectedValue(new Error()); const call = () => diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts index 9eb9ff70f3f..26338a1c095 100644 --- a/frontend/src/tests/lib/constants/environment.constants.spec.ts +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -27,4 +27,20 @@ describe("FEATURE_FLAG_ENVIRONMENT", () => { ); expect(FEATURE_FLAG_ENVIRONMENT).toEqual(defaultFeatureFlagValues); }); + + it("should fallback to default on error", async () => { + const spyConsoleError = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); + vi.spyOn(envVarsUtils, "getEnvVars").mockReturnValue({ + ...environmentVars, + featureFlags: `{"TEST_FLAG_NOT_EDITABLE": TRUE}`, + }); + + const { FEATURE_FLAG_ENVIRONMENT } = await import( + "$lib/constants/environment.constants" + ); + expect(FEATURE_FLAG_ENVIRONMENT).toEqual(defaultFeatureFlagValues); + expect(spyConsoleError).toBeCalledTimes(1); + }); }); From 31f926b3fc93433310665af42e2aeb3006f0655b Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Thu, 21 Nov 2024 14:52:01 +0100 Subject: [PATCH 7/8] Comments --- frontend/src/tests/lib/constants/environment.constants.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts index 26338a1c095..2197fd20ab9 100644 --- a/frontend/src/tests/lib/constants/environment.constants.spec.ts +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -5,6 +5,9 @@ describe("FEATURE_FLAG_ENVIRONMENT", () => { const environmentVars = envVarsUtils.getEnvVars(); beforeEach(() => { + // The FEATURE_FLAG_ENVIRONMENT is a constant that is set once when the module + // `environment.constants` is imported. To test different states of it, + // we need to reset the imported modules and reimport `environment.constants` for each test. vi.resetModules(); }); From f0757abd5a398f6cea85f873d0aba91b313a31f3 Mon Sep 17 00:00:00 2001 From: Max Strasinsky Date: Fri, 22 Nov 2024 09:37:45 +0100 Subject: [PATCH 8/8] test: expect error argument --- .../src/tests/lib/constants/environment.constants.spec.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frontend/src/tests/lib/constants/environment.constants.spec.ts b/frontend/src/tests/lib/constants/environment.constants.spec.ts index 2197fd20ab9..8dc789b283c 100644 --- a/frontend/src/tests/lib/constants/environment.constants.spec.ts +++ b/frontend/src/tests/lib/constants/environment.constants.spec.ts @@ -45,5 +45,9 @@ describe("FEATURE_FLAG_ENVIRONMENT", () => { ); expect(FEATURE_FLAG_ENVIRONMENT).toEqual(defaultFeatureFlagValues); expect(spyConsoleError).toBeCalledTimes(1); + expect(spyConsoleError).toBeCalledWith( + "Error parsing featureFlags", + new SyntaxError("Unexpected token T in JSON at position 27") + ); }); });