From 07bbe8448b2a63a6b22cb4b9670fa37f984ee5c8 Mon Sep 17 00:00:00 2001 From: Yongjie Zhao Date: Tue, 14 Dec 2021 16:19:55 +0800 Subject: [PATCH] refactor(monorepo): change coverage of core to 100% (#17698) --- .codecov.yml | 2 +- superset-frontend/jest.config.js | 7 +-- .../src/color/CategoricalColorNamespace.ts | 7 +-- .../src/connection/SupersetClientClass.ts | 15 +++--- .../superset-ui-core/src/utils/random.ts | 9 ++-- .../connection/SupersetClientClass.test.ts | 49 +++++++++++++++---- .../test/translation/Translator.test.ts | 6 +-- .../translation/TranslatorSingleton.test.ts | 21 +++++--- .../test/translation/index.test.ts | 2 +- .../test/translation/languagePacks/en.ts | 2 +- .../test/translation/languagePacks/zh.ts | 2 +- .../test/utils/logging.test.ts | 39 ++++++++------- 12 files changed, 95 insertions(+), 66 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 4d86439e9969d..149042367ab18 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -16,7 +16,7 @@ coverage: target: auto threshold: 0% core-packages-ts: - target: 95% + target: 100% paths: - 'superset-frontend/packages' - '!superset-frontend/packages/**/*.jsx' diff --git a/superset-frontend/jest.config.js b/superset-frontend/jest.config.js index bc961ad3d16ce..90af1ee6cd343 100644 --- a/superset-frontend/jest.config.js +++ b/superset-frontend/jest.config.js @@ -56,9 +56,10 @@ module.exports = { coverageReporters: ['lcov', 'json-summary', 'html'], transform: { '^.+\\.jsx?$': 'babel-jest', - // ts-jest can't load plugin 'babel-plugin-typescript-to-proptypes' - 'reactify\\.tsx$': 'babel-jest', - '^.+\\.tsx?$': 'ts-jest', + // ts-jest doesn't work with `--coverage`. @superset-ui/core should + // 100% coverage, so we use babel-jest in packages and plugins. + '(plugins|packages)\\/.+\\.tsx?$': 'babel-jest', + '(((?!(plugins|packages)).)*)\\/.+\\.tsx?$': 'ts-jest', }, moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json'], snapshotSerializers: ['@emotion/jest/enzyme-serializer'], diff --git a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts index 7e28cae6babac..9c56d5114b9d9 100644 --- a/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts +++ b/superset-frontend/packages/superset-ui-core/src/color/CategoricalColorNamespace.ts @@ -40,12 +40,7 @@ export default class CategoricalColorNamespace { getScale(schemeId?: string) { const id = schemeId ?? getCategoricalSchemeRegistry().getDefaultKey() ?? ''; const scheme = getCategoricalSchemeRegistry().get(id); - const newScale = new CategoricalColorScale( - scheme?.colors ?? [], - this.forcedItems, - ); - - return newScale; + return new CategoricalColorScale(scheme?.colors ?? [], this.forcedItems); } /** diff --git a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts index 4959fb1bf2d5b..ef52134f31376 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/SupersetClientClass.ts @@ -33,13 +33,6 @@ import { } from './types'; import { DEFAULT_FETCH_RETRY_OPTIONS, DEFAULT_BASE_URL } from './constants'; -function redirectUnauthorized() { - // the next param will be picked by flask to redirect the user after the login - setTimeout(() => { - window.location.href = `/login?next=${window.location.href}`; - }); -} - export default class SupersetClientClass { credentials: Credentials; @@ -159,8 +152,8 @@ export default class SupersetClientClass { timeout: timeout ?? this.timeout, fetchRetryOptions: fetchRetryOptions ?? this.fetchRetryOptions, }).catch(res => { - if (res && res.status === 401) { - redirectUnauthorized(); + if (res?.status === 401) { + this.redirectUnauthorized(); } return Promise.reject(res); }); @@ -226,4 +219,8 @@ export default class SupersetClientClass { endpoint[0] === '/' ? endpoint.slice(1) : endpoint }`; } + + redirectUnauthorized() { + window.location.href = `/login?next=${window.location.href}`; + } } diff --git a/superset-frontend/packages/superset-ui-core/src/utils/random.ts b/superset-frontend/packages/superset-ui-core/src/utils/random.ts index 38dbdbd10bba2..0edddfd178a31 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/random.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/random.ts @@ -16,15 +16,12 @@ * specific language governing permissions and limitations * under the License. */ -import seedrandom from 'seedrandom'; - -let random = seedrandom('superset-ui'); +import _seedrandom from 'seedrandom'; export function seed(seed: string) { - random = seedrandom(seed); - return random; + return _seedrandom(seed); } export function seedRandom() { - return random(); + return _seedrandom('superset-ui')(); } diff --git a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts index 2edb0f46df83f..4b2307c9d4f26 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts @@ -17,10 +17,7 @@ * under the License. */ import fetchMock from 'fetch-mock'; -import { - SupersetClientClass, - ClientConfig, -} from '@superset-ui/core/src/connection'; +import { SupersetClientClass, ClientConfig, CallApi } from '@superset-ui/core'; import { LOGIN_GLOB } from './fixtures/constants'; describe('SupersetClientClass', () => { @@ -321,7 +318,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.get({ url: mockGetUrl }); - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(clientConfig.mode); expect(fetchRequest.credentials).toBe(clientConfig.credentials); expect(fetchRequest.headers).toEqual( @@ -378,7 +375,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.get({ url: mockGetUrl, ...overrideConfig }); - const fetchRequest = fetchMock.calls(mockGetUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockGetUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); expect(fetchRequest.headers).toEqual( @@ -423,7 +420,7 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, ...overrideConfig }); - const fetchRequest = fetchMock.calls(mockPostUrl)[0][1]; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; expect(fetchRequest.mode).toBe(overrideConfig.mode); expect(fetchRequest.credentials).toBe(overrideConfig.credentials); @@ -454,7 +451,8 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, postPayload }); - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; + const formData = fetchRequest.body as FormData; expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); Object.entries(postPayload).forEach(([key, value]) => { @@ -470,7 +468,8 @@ describe('SupersetClientClass', () => { await client.init(); await client.post({ url: mockPostUrl, postPayload, stringify: false }); - const formData = fetchMock.calls(mockPostUrl)[0][1].body as FormData; + const fetchRequest = fetchMock.calls(mockPostUrl)[0][1] as CallApi; + const formData = fetchRequest.body as FormData; expect(fetchMock.calls(mockPostUrl)).toHaveLength(1); Object.entries(postPayload).forEach(([key, value]) => { @@ -479,4 +478,36 @@ describe('SupersetClientClass', () => { }); }); }); + + it('should redirect Unauthorized', async () => { + const mockRequestUrl = 'https://host/get/url'; + const { location } = window; + // @ts-ignore + delete window.location; + // @ts-ignore + window.location = { href: mockRequestUrl }; + const authSpy = jest + .spyOn(SupersetClientClass.prototype, 'ensureAuth') + .mockImplementation(); + const rejectValue = { status: 401 }; + fetchMock.get(mockRequestUrl, () => Promise.reject(rejectValue), { + overwriteRoutes: true, + }); + + const client = new SupersetClientClass({}); + + let error; + try { + await client.request({ url: mockRequestUrl, method: 'GET' }); + } catch (err) { + error = err; + } finally { + const redirectURL = window.location.href; + expect(redirectURL).toBe(`/login?next=${mockRequestUrl}`); + expect(error.status).toBe(401); + } + + authSpy.mockReset(); + window.location = location; + }); }); diff --git a/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts index bab9081a7cdb0..703d9a5d309c0 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/Translator.test.ts @@ -17,16 +17,16 @@ * under the License. */ -import { logging } from '@superset-ui/core'; -import Translator from '@superset-ui/core/src/translation/Translator'; import { + logging, configure, t, tn, addLocaleData, addTranslation, addTranslations, -} from '@superset-ui/core/src/translation/TranslatorSingleton'; +} from '@superset-ui/core'; +import Translator from '../../src/translation/Translator'; import languagePackZh from './languagePacks/zh'; import languagePackEn from './languagePacks/en'; diff --git a/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts index 24ccd260d4fa1..af0d6f5915524 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/TranslatorSingleton.test.ts @@ -19,13 +19,8 @@ /* eslint no-console: 0 */ import mockConsole from 'jest-mock-console'; -import Translator from '@superset-ui/core/src/translation/Translator'; -import { - configure, - resetTranslation, - t, - tn, -} from '@superset-ui/core/src/translation/TranslatorSingleton'; +import { configure, resetTranslation, t, tn } from '@superset-ui/core'; +import Translator from '../../src/translation/Translator'; import languagePackEn from './languagePacks/en'; import languagePackZh from './languagePacks/zh'; @@ -76,4 +71,16 @@ describe('TranslatorSingleton', () => { }); }); }); + it('should be reset translation setting', () => { + configure(); + expect(t('second')).toEqual('second'); + + resetTranslation(); + const restoreConsole = mockConsole(); + expect(t('second')).toEqual('second'); + resetTranslation(); + expect(t('second')).toEqual('second'); + expect(console.warn).toBeCalledTimes(2); + restoreConsole(); + }); }); diff --git a/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts b/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts index aecc3e6fafcfb..2c0f0d6f92d5b 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/index.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { configure, t, tn } from '@superset-ui/core/src/translation'; +import { configure, t, tn } from '@superset-ui/core'; describe('index', () => { it('exports configure()', () => { diff --git a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts index 0efcc12645bcd..00d52be8a2728 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/en.ts @@ -17,7 +17,7 @@ * under the License. */ -import { LanguagePack } from '@superset-ui/core/src/translation'; +import { LanguagePack } from '@superset-ui/core'; const languagePack: LanguagePack = { domain: 'superset', diff --git a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts index 105a65431e564..420a2bda1e417 100644 --- a/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts +++ b/superset-frontend/packages/superset-ui-core/test/translation/languagePacks/zh.ts @@ -17,7 +17,7 @@ * under the License. */ -import { LanguagePack } from '@superset-ui/core/src/translation'; +import { LanguagePack } from '@superset-ui/core'; const languagePack: LanguagePack = { domain: 'superset', diff --git a/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts b/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts index c5e158c946355..cdf4df89cf377 100644 --- a/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/utils/logging.test.ts @@ -21,40 +21,41 @@ describe('logging', () => { beforeEach(() => { jest.resetModules(); - // Explicit is better than implicit - console.warn = console.error = function mockedConsole(message) { - throw new Error(message); - }; + jest.resetAllMocks(); }); + it('should pipe to `console` methods', () => { - const { logging } = require('@superset-ui/core/src'); + const { logging } = require('@superset-ui/core'); + jest.spyOn(logging, 'debug').mockImplementation(); + jest.spyOn(logging, 'log').mockImplementation(); + jest.spyOn(logging, 'info').mockImplementation(); expect(() => { logging.debug(); logging.log(); logging.info(); }).not.toThrow(); - expect(() => { - logging.warn('warn'); - }).toThrow('warn'); - expect(() => { - logging.error('error'); - }).toThrow('error'); - // to support: npx jest --silent - const spy = jest.spyOn(logging, 'trace'); - spy.mockImplementation(() => { + jest.spyOn(logging, 'warn').mockImplementation(() => { + throw new Error('warn'); + }); + expect(() => logging.warn()).toThrow('warn'); + + jest.spyOn(logging, 'error').mockImplementation(() => { + throw new Error('error'); + }); + expect(() => logging.error()).toThrow('error'); + + jest.spyOn(logging, 'trace').mockImplementation(() => { throw new Error('Trace:'); }); - expect(() => { - logging.trace(); - }).toThrow('Trace:'); - spy.mockRestore(); + expect(() => logging.trace()).toThrow('Trace:'); }); + it('should use noop functions when console unavailable', () => { const { console } = window; Object.assign(window, { console: undefined }); - const { logging } = require('@superset-ui/core/src'); + const { logging } = require('@superset-ui/core'); afterAll(() => { Object.assign(window, { console });