Skip to content
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 src/setupTests.js to specify environment setup for Jest (#545) #548

Merged
merged 1 commit into from
Sep 2, 2016

Conversation

gaelollivier
Copy link
Contributor

@gaelollivier gaelollivier commented Sep 2, 2016

Fix #545

Test plan

Run npm run create-react-app my-app and in the generated App.js, add something like localStorage.getItem('test');. Run npm test, it should fail because localStorage is undefined. Add a setupTests.js in src that looks like this:

const localStorageMock = {
  getItem: jest.fn(),
  setItem: jest.fn(),
  clear: jest.fn()
};
global.localStorage = localStorageMock

Then run npm test again (you need to re-run the command in order to have the setupTests.js added to Jest config) and it should pass.
You can also simply add a setupTests.js that does a console.log('Hello world') to check it's properly executed before the tests.

@ghost
Copy link

ghost commented Sep 2, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Can you please specify the test plan?

See https://github.com/facebookincubator/create-react-app/blob/master/CONTRIBUTING.md#submitting-a-pull-request:

Please also provide a test plan, i.e. specify how you verified that your addition works.

@ghost
Copy link

ghost commented Sep 2, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ghost ghost added the CLA Signed label Sep 2, 2016
@gaelollivier
Copy link
Contributor Author

gaelollivier commented Sep 2, 2016

Test plan added to the initial PR description:

Test plan

Run npm run create-react-app my-app. The template setupTests.js is added in src. Any code in this file should be executed when you run your tests via npm test (for example, add a console.log("Hello world!");, this should be printed in the tests report). If you remove the file, npm test still works as before.

@@ -0,0 +1,4 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we shouldn’t ship this by default. It’s a little bit advanced feature, and it’s fine to just document it. Can you add relevant documentation section to template/README.md instead?

@gaearon gaearon added this to the 0.4.0 milestone Sep 2, 2016
@ghost ghost added the CLA Signed label Sep 2, 2016
@ghost ghost added the CLA Signed label Sep 2, 2016
@gaelollivier
Copy link
Contributor Author

Removed the sample 'setupTests.js' and added docs to the README.md

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Sorry for the trouble but I just merged another set of changes that made this conflict.
Could you please rebase?

@@ -671,6 +672,22 @@ import { expect } from 'chai';

and then use them in your tests like you normally do.

### Test environment setup
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s call this ### Initializing Test Environment

@gaelollivier
Copy link
Contributor Author

Fixed doc title and conflicts ✅

@ghost ghost added the CLA Signed label Sep 2, 2016
@@ -674,6 +675,22 @@ import { expect } from 'chai';

and then use them in your tests like you normally do.

### Initializing Test Environment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add “>Note: this feature is available since react-scripts@0.4.0” here. You can copy and paste an existing similar notes and change the version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Awesome. Thank you.

const config = {
moduleNameMapper: {
'^[./a-zA-Z0-9$_-]+\\.(jpg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm)$': resolve('config/jest/FileStub.js'),
'^[./a-zA-Z0-9$_-]+\\.css$': resolve('config/jest/CSSStub.js')
},
scriptPreprocessor: resolve('config/jest/transform.js'),
setupFiles: [resolve('config/polyfills.js')],
setupFiles: setupFiles,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you don't need the : setupFiles piece here because of object short notation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure Node 4 supports it, does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, wasn't sure of what es6 features I could use there since there are some "var" elsewhere in the code :s

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it does! Node 4 supports let/var as well as object short notation and arrow functions.

@gaearon
Copy link
Contributor

gaearon commented Sep 2, 2016

Out in 0.4.0.
See the release notes!

stayradiated pushed a commit to stayradiated/create-react-app that referenced this pull request Sep 7, 2016
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants