-
Notifications
You must be signed in to change notification settings - Fork 13.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(monorepo): change coverage of core to 100% #17698
Changes from 6 commits
b9100fe
3ed0026
af2695c
1f03c67
9403c68
8fb761c
bf94383
e0f198d
cc78560
980ea40
0463ee6
fda8acf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. reformat |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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')(); | ||
Comment on lines
-19
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor refactor, to avoid duplicate names(seedrandom). |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'); | ||
Comment on lines
+30
to
-42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mock console output, to make clear test output. before
After
|
||
|
||
// 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 }); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️