-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Improvements in setup of jest runner. Update GettingStarted documentation. #329
Conversation
We should probably keep the same patterns with defining E2E test like unit in jest (talking about __tests__ pattern). Also setup can be in one file if we conditionally load `detox` object. How do you feel about this? :)
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.
Thank you for taking the time, I like a lot of this PR!
I added some comments, I hope we can discuss about that ;)
docs/Guide.Jest.md
Outdated
await detox.init(config); | ||
}); | ||
// setup detox only when running e2e tests | ||
if (process.argv[2].includes('__e2e__')) { |
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 not needed when we specify the setup files, which I think is a cleaner approach
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.
I thought about one file of setup for all environment (including unit tests). But we can run this file specifically for e2e tests as you mention :)
docs/Guide.Jest.md
Outdated
``` | ||
|
||
### 4. Run jest | ||
|
||
Add this part to your `package.json`: | ||
```json | ||
"jest": { | ||
"preset": "react-native", |
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.
You don't need this jest config as we run on the build app
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.
As above, I had in my mind to create a unified setup for unit and e2e but You are correct, for e2e test we do not need it :)
docs/Guide.Jest.md
Outdated
@@ -19,32 +19,39 @@ You should remove `e2e/mocha.opts`, you no longer need it. | |||
### 3. Write a detox setup file | |||
|
|||
```js | |||
// ./jest/setup/ |
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.
I like this part 👍
docs/Guide.Jest.md
Outdated
"scripts": { | ||
"test:e2e": "jest e2e --setupTestFrameworkScriptFile=./jest/setup-e2e-tests.js --runInBand" | ||
"test:e2e": "detox build && jest __e2e__ --runInBand" |
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.
The build part is left out because with detox test
it isn't included either, but adding something like test:e2e:build: "detox build"
is a pretty neat idea!
I've applied some changes to this one @DanielMSchmidt. If anything more is bothering You add more notes :) |
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.
Looks great, thank you so much for taking the time!
We should probably keep the same patterns with defining E2E test like unit in jest (talking about tests pattern). Also setup can be in one file if we conditionally load
detox
object. How do you feel about this? :) @DanielMSchmidt