-
Notifications
You must be signed in to change notification settings - Fork 111
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
adds modifyBabelConfig #235
Changes from 11 commits
ad7f114
931af3b
350c1a0
00ffded
db9dce1
835f0a3
3b57e11
5e3dad7
5c51659
bb9c7be
b789da3
52965aa
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 |
---|---|---|
@@ -0,0 +1,23 @@ | ||
const fakeTransformer = {}; | ||
|
||
jest.setMock('babel-jest', { | ||
createTransformer: jest.fn(() => fakeTransformer), | ||
}); | ||
const babelJest = require('babel-jest'); | ||
|
||
const fakeBabelResult = {}; | ||
|
||
jest.setMock('../../../config/babel', jest.fn(() => fakeBabelResult)); | ||
const babel = require('../../../config/babel'); | ||
|
||
describe('preprocessor', () => { | ||
it('calls the babel utility and babel-jest\'s createTransformer', () => { | ||
const preprocessor = require('../preprocessor'); // eslint-disable-line global-require | ||
expect(preprocessor).toBe(fakeTransformer); | ||
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. Isn't this just testing Jest's ability to mock? 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. ha yeah I suppose, I'll drop this assertion 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. although, wait, I added it to verify that 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. Ah ok gotcha! |
||
expect(babel.mock.calls[0][0]).toEqual({ | ||
type: 'test', | ||
environment: 'test', | ||
}); | ||
expect(babelJest.createTransformer).toBeCalledWith(fakeBabelResult); | ||
}); | ||
}); | ||
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. Thanks for adding this test!! 🙏 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,10 @@ | ||
const mergeAll = require('ramda').mergeAll; | ||
const babel = require('../../config/babel')(); | ||
const babelJest = require('babel-jest'); | ||
const buildConfigs = require('../buildConfigs'); | ||
const config = require('../kytConfig')(); | ||
|
||
const { clientConfig } = buildConfigs(config); | ||
const options = { | ||
type: 'test', | ||
environment: 'test', | ||
}; | ||
|
||
// Merge userland babel config with our babel config | ||
// This should go away after https://github.com/NYTimes/kyt/issues/134 | ||
const clientBabelConfig = clientConfig.module.loaders | ||
.find(loader => loader.loader === 'babel-loader') | ||
.query; | ||
|
||
const babelConfigForJest = mergeAll([{}, babel, clientBabelConfig]); | ||
const babelConfigForJest = require('../../config/babel')(options); | ||
|
||
module.exports = babelJest.createTransformer(babelConfigForJest); |
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.
This is a misnomer right? There's no actual merging going on.
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.
yeah, maybe
modifiedBabelConfig
would be more accurate?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.
Yea that sounds good
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.
@tizmagik - done