From 347fff04267137e2cb0ad41aaede0b4db7c8b1ef Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 3 Nov 2022 11:23:29 +0800 Subject: [PATCH 1/3] refactor: feature flag getter --- .../src/utils/featureFlags.ts | 9 +-- .../test/utils/featureFlag.test.ts | 58 ++++++++----------- 2 files changed, 27 insertions(+), 40 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts index 380704f0180b6..4eb90ea0afffa 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/featureFlags.ts @@ -87,14 +87,11 @@ declare global { } } -export function isFeatureEnabled(feature: FeatureFlag) { +export function isFeatureEnabled(feature: FeatureFlag): boolean { try { return !!window.featureFlags[feature]; } catch (error) { - // eslint-disable-next-line no-console - console.error(`Failed to query feature flag ${feature} (see error below)`); - // eslint-disable-next-line no-console - console.error(error); - return false; + console.error(`Failed to query feature flag ${feature}`); } + return false; } diff --git a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts index b2a273d2ed78b..0da76435a6a95 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts @@ -16,48 +16,38 @@ * specific language governing permissions and limitations * under the License. */ - -import { FeatureFlag, isFeatureEnabled } from '@superset-ui/core'; - -const originalFeatureFlags = window.featureFlags; -// eslint-disable-next-line no-console -const originalConsoleError = console.error; -const reset = () => { - window.featureFlags = originalFeatureFlags; - // eslint-disable-next-line no-console - console.error = originalConsoleError; -}; +import mockConsole from 'jest-mock-console'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; it('returns false and raises console error if feature flags have not been initialized', () => { - // eslint-disable-next-line no-console - console.error = jest.fn(); - delete (window as any).featureFlags; - expect(isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING)).toEqual( - false, - ); - - // eslint-disable-next-line no-console - expect(console.error).toHaveBeenNthCalledWith( - 1, - 'Failed to query feature flag ALLOW_DASHBOARD_DOMAIN_SHARDING (see error below)', - ); - - reset(); + const restoreConsole = mockConsole(); + Object.defineProperty(window, 'featureFlags', { + value: undefined, + }); + + expect( + isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING), + ).toBeFalsy(); + expect(console.error).toHaveBeenCalled(); + restoreConsole(); }); it('returns false for unset feature flag', () => { - expect(isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING)).toEqual( - false, - ); + Object.defineProperty(window, 'featureFlags', { + value: {}, + }); - reset(); + expect( + isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING), + ).toBeFalsy(); }); it('returns true for set feature flag', () => { - window.featureFlags = { - [FeatureFlag.CLIENT_CACHE]: true, - }; + Object.defineProperty(window, 'featureFlags', { + value: { + CLIENT_CACHE: true, + }, + }); - expect(isFeatureEnabled(FeatureFlag.CLIENT_CACHE)).toEqual(true); - reset(); + expect(isFeatureEnabled(FeatureFlag.CLIENT_CACHE)).toBeTruthy(); }); From febbdc6a3cd2228be244408b1562086b3b5e5445 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 3 Nov 2022 11:50:02 +0800 Subject: [PATCH 2/3] error context --- .../superset-ui-core/test/utils/featureFlag.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts index 0da76435a6a95..bc3ce70b2fd2d 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts @@ -20,7 +20,7 @@ import mockConsole from 'jest-mock-console'; import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; it('returns false and raises console error if feature flags have not been initialized', () => { - const restoreConsole = mockConsole(); + mockConsole(); Object.defineProperty(window, 'featureFlags', { value: undefined, }); @@ -29,7 +29,10 @@ it('returns false and raises console error if feature flags have not been initia isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING), ).toBeFalsy(); expect(console.error).toHaveBeenCalled(); - restoreConsole(); + // @ts-expect-error + expect(console.error.mock.calls[0][0]).toEqual( + 'Failed to query feature flag ALLOW_DASHBOARD_DOMAIN_SHARDING', + ); }); it('returns false for unset feature flag', () => { From b7eb30fc49c5e47a76445e290f70ee50572b03e0 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Thu, 3 Nov 2022 15:47:41 +0800 Subject: [PATCH 3/3] update assertion --- .../test/utils/featureFlag.test.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts index bc3ce70b2fd2d..66c58e79af567 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/featureFlag.test.ts @@ -25,9 +25,9 @@ it('returns false and raises console error if feature flags have not been initia value: undefined, }); - expect( - isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING), - ).toBeFalsy(); + expect(isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING)).toEqual( + false, + ); expect(console.error).toHaveBeenCalled(); // @ts-expect-error expect(console.error.mock.calls[0][0]).toEqual( @@ -40,9 +40,9 @@ it('returns false for unset feature flag', () => { value: {}, }); - expect( - isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING), - ).toBeFalsy(); + expect(isFeatureEnabled(FeatureFlag.ALLOW_DASHBOARD_DOMAIN_SHARDING)).toEqual( + false, + ); }); it('returns true for set feature flag', () => { @@ -52,5 +52,5 @@ it('returns true for set feature flag', () => { }, }); - expect(isFeatureEnabled(FeatureFlag.CLIENT_CACHE)).toBeTruthy(); + expect(isFeatureEnabled(FeatureFlag.CLIENT_CACHE)).toEqual(true); });