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

Test environment gets current file path as option #7442

Merged
merged 1 commit into from Dec 19, 2018

Conversation

avaly
Copy link
Contributor

@avaly avaly commented Nov 30, 2018

Summary

The testEnvironment feature enables a whole new suite of tests to be written with Jest.

However the following use case can present itself:

  1. an application has an async setup with involves a connection task and a resource bootstrapping task (e.g. prepare some external resources).
  2. the application's connection task is slow, but is safe to run several times (since it has already initialized connections to the external resources) - so running this in a setupFiles is less than optimal, as we would pay the price for this task on each test file
  3. the application's bootstrap task needs to be configurable per test file - we currently don't have any option to do this.
  4. the application's bootstrap task can not run in the test file context, because the connection task was not run in the same context (due to the time penalty described in 2.)

PS: Related #6889

Test plan

$ node ./packages/jest-cli/bin/jest.js test_environment_async
 PASS  e2e/__tests__/test_environment_async.test.js
  ✓ triggers setup/teardown hooks (861ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.288s, estimated 2s
Ran all test suites matching /test_environment_async/i.

@facebook-github-bot
Copy link
Contributor

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired.

Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status.

@SimenB
Copy link
Member

SimenB commented Nov 30, 2018

Not sure about the feature, but it should be passed in the constructor, not the individual methods

@avaly avaly changed the title Call environment setup/teardown with current path Test environment gets current file path as option Dec 1, 2018
docs/Configuration.md Outdated Show resolved Hide resolved
Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

Still want to digest this, but I left a few comments in the mean time

docs/Configuration.md Outdated Show resolved Hide resolved
const environment = new TestEnvironment(config, {console: testConsole});
const environment = new TestEnvironment(config, {
console: testConsole,
testPath: path,
Copy link
Member

Choose a reason for hiding this comment

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

Can you think of anything else we should expose here while we're thinking about it?

@avaly avaly force-pushed the feature/test-environment-path branch 2 times, most recently from 69bcc05 to 92f0050 Compare December 7, 2018 10:33
@facebook-github-bot
Copy link
Contributor

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

@SimenB
Copy link
Member

SimenB commented Dec 11, 2018

We're getting a flow error, mind taking a look at it? Run yarn flow locally to see it

@avaly avaly force-pushed the feature/test-environment-path branch from 40d4a83 to 7c18f46 Compare December 12, 2018 09:23
@avaly
Copy link
Contributor Author

avaly commented Dec 12, 2018

Rebased and fixed Flow error.

@codecov-io
Copy link

Codecov Report

Merging #7442 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7442      +/-   ##
==========================================
- Coverage   67.46%   67.44%   -0.02%     
==========================================
  Files         247      247              
  Lines        9516     9511       -5     
  Branches        5        5              
==========================================
- Hits         6420     6415       -5     
  Misses       3094     3094              
  Partials        2        2
Impacted Files Coverage Δ
packages/jest-environment-jsdom/src/index.js 39.02% <ø> (ø) ⬆️
packages/jest-runner/src/runTest.js 3.07% <0%> (ø) ⬆️
packages/jest-config/src/getMaxWorkers.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df78255...7c18f46. Read the comment docs.

@SimenB SimenB merged commit 9208ac6 into jestjs:master Dec 19, 2018
@avaly avaly deleted the feature/test-environment-path branch December 19, 2018 21:41
captain-yossarian pushed a commit to captain-yossarian/jest that referenced this pull request Jul 18, 2019
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
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.

None yet

6 participants