-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Add Jest. #250
Add Jest. #250
Changes from all commits
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 |
---|---|---|
|
@@ -3,3 +3,4 @@ build | |
.DS_Store | ||
*.tgz | ||
my-app* | ||
template/src/__tests__/__snapshots__/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ module.exports = { | |
browser: true, | ||
commonjs: true, | ||
es6: true, | ||
jest: true, | ||
node: true | ||
}, | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
* | ||
* @flow | ||
*/ | ||
|
||
module.exports = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
* | ||
* @flow | ||
*/ | ||
|
||
module.exports = "test-file-stub"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
const babelDev = require('../babel.dev'); | ||
const babelJest = require('babel-jest'); | ||
|
||
module.exports = babelJest.createTransformer(babelDev); | ||
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. this seems like the nicest way to ensure low-maintenance of this script as we add more stuff to babel-jest. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -87,7 +87,10 @@ function createApp(name, verbose, version) { | |
version: '0.0.1', | ||
private: true, | ||
}; | ||
fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify(packageJson)); | ||
fs.writeFileSync( | ||
path.join(root, 'package.json'), | ||
JSON.stringify(packageJson, null, 2) | ||
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. this doesn't really matter so much but the initial version of package.json wasn't formatted nicely. |
||
); | ||
var originalDirectory = process.cwd(); | ||
process.chdir(root); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
process.env.NODE_ENV = 'test'; | ||
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. Would you mind documenting the behavior of NODE_ENV somewhere like in the README? I know you didn't start using it, but it seems to be undocumented at the moment. 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. It's not affecting anything in Jest itself but it is kind of a standard that people have adopted. For example, Relay's preprocessor behaves differently when this flag is set. Jest sets this in its binary (jest-cli/bin/jest.js) but since we are not it directly here we have to add it. |
||
|
||
const createJestConfig = require('./utils/create-jest-config'); | ||
const jest = require('jest'); | ||
const path = require('path'); | ||
const paths = require('../config/paths'); | ||
|
||
const argv = process.argv.slice(2); | ||
|
||
const index = argv.indexOf('--debug-template'); | ||
if (index !== -1) { | ||
argv.splice(index, 1); | ||
} | ||
|
||
argv.push('--config', JSON.stringify(createJestConfig( | ||
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. seems like we pass the config explicitly again -- is it because Jest can't use the config in 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. Jest could but I specifically asked that we avoid polluting package.json with config options :-) |
||
relativePath => path.resolve(__dirname, '..', relativePath), | ||
path.resolve(paths.appSrc, '..') | ||
))); | ||
|
||
jest.run(argv); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
/** | ||
* Copyright (c) 2015-present, Facebook, Inc. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
module.exports = (resolve, rootDir) => { | ||
const config = { | ||
automock: false, | ||
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. I really like that this turns off automocking by default. Do you think Jest itself should / will soon switch that to the default behavior for everyone? It might confuse people when, if they start with 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. i really like that too. 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. It's been my plan to slowly move away from automocking as a default and towards explicit mocks using Let's do it for Jest 15, which should be out in less than a month or so? |
||
moduleNameMapper: { | ||
'^[./a-zA-Z0-9$_-]+\\.(jpg|png|gif|eot|svg|ttf|woff|woff2|mp4|webm)$': resolve('config/jest/FileStub.js'), | ||
'^[./a-zA-Z0-9$_-]+\\.css$': resolve('config/jest/CSSStub.js') | ||
}, | ||
persistModuleRegistryBetweenSpecs: true, | ||
scriptPreprocessor: resolve('config/jest/transform.js'), | ||
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. Shouldn't these require paths be relative? I'm confused as to how this is currently able to find the 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. It resolves them either to |
||
setupFiles: [ | ||
resolve('config/polyfills.js') | ||
], | ||
testEnvironment: 'node' | ||
}; | ||
if (rootDir) { | ||
config.rootDir = rootDir; | ||
} | ||
return config; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import React from 'react'; | ||
import App from '../App'; | ||
import renderer from 'react-test-renderer'; | ||
|
||
describe('App', () => { | ||
it('renders a welcome view', () => { | ||
const instance = renderer.create(<App />); | ||
const tree = instance.toJSON(); | ||
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. Is this API going to change? I remember there were talks about not wanting people to inspect the JSON. 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. We are likely to add more APIs to the instance to do selections (similar to enzyme, except component based, like |
||
expect(tree).toMatchSnapshot(); | ||
}); | ||
}); |
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.
Can you also add a
test
command to mainpackage.json
that would work with--debug-template
? Please see how we switch paths inconfig/paths.js
. This lets us iterate super quickly on the template without runningcreate-react-app
every time we want to see if the commands still work.You can rename the existing
test
command toe2e
.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.
5652ad5
is this what you had in mind? Because
test
doesn't have a script associated with it I wrote a quick script to add a package.json that links to the preset, runs the test and ensures a snapshot file was created.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.
Hmm I'm a bit confused by this.
We only use shell scripts for our own infrastructure (end to end test and release script). We would like all user-ran scripts to be written in JavaScript.
Ideally I'd like to mirror what we do with start and build. Create a JS file in scripts directory called "test.js". It's fine if all it does is call Jest.
Then set that script as "test" command in package.json, identical to how we do build and start. This lets us work on Create React App locally without creating apps all the time. We just run those commands in the root folder, and they work (try npm start).
Finally, npm test needs to be a part of end to end test. To be clear e2e is our test that ensures Create React App works. It runs npm start and npm run build in root folder to test our development environment, then creates an app and runs them again to make sure generated app works, then ejects and runs them yet again. So this script is where you want to add "npm test" calls so that we know that testing infrastructure works and won't be broken by future PRs.