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.

Sign up for GitHub

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

Jump to bottom

move maxWorkers to globalConfig #4005

Merged
merged 1 commit into from
Jul 12, 2017
Merged

move maxWorkers to globalConfig #4005

merged 1 commit into from
Jul 12, 2017

Conversation

aaronabramov
Copy link
Contributor

No description provided.

@aaronabramov aaronabramov force-pushed the max_workers_to_global_config branch 2 times, most recently from ecbe48d to 6a3d1ae Compare July 11, 2017 05:01
@cpojer
Copy link
Member

cpojer commented Jul 11, 2017

Make sure CI passes.

@aaronabramov aaronabramov force-pushed the max_workers_to_global_config branch 3 times, most recently from eb42f02 to 1500617 Compare July 11, 2017 17:14
thymikee
thymikee reviewed Jul 11, 2017
View reviewed changes
packages/jest-cli/src/test_runner.js Outdated
@@ -205,6 +204,7 @@ class TestRunner {
}

this._dispatcher.onTestStart(test);
console.error(test.path);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i just added it to debug appveyor :(

@thymikee
Copy link
Collaborator

We can get rid of options here as maxWorkers can now be accessed through globalConfig

@aaronabramov
Copy link
Contributor Author

@thymikee oh.. that's a good catch, there's the whole reporters options object that can be removed too. Strange that flow didn't catch it

@thymikee
Copy link
Collaborator

I would be surprised if it had caught that 😄. Doesn't seem to be trivial to infer (programatically) that second argument is unnecessary, because it can be calculated from first one. Warnings like that would be gold though.

@aaronabramov aaronabramov force-pushed the max_workers_to_global_config branch from 1500617 to 918e75d Compare July 12, 2017 17:29
@aaronabramov aaronabramov force-pushed the max_workers_to_global_config branch from 918e75d to 5a57c1a Compare July 12, 2017 17:37
@aaronabramov
Copy link
Contributor Author

seems to be one integration test snapshot.test.js that timeouts on windows (it passes when i try to login to appveyor with RDP). I dispabled it.

@thymikee i already started cleaning up the reporters options in the next pr (#4012) i'll finish it there to avoid resolving conflicts :)

@codecov-io
Copy link

Codecov Report

Merging #4005 into master will increase coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4005      +/-   ##
==========================================
+ Coverage   60.26%   60.28%   +0.02%     
==========================================
  Files         196      196              
  Lines        6765     6764       -1     
  Branches        6        6              
==========================================
+ Hits         4077     4078       +1     
+ Misses       2685     2683       -2     
  Partials        3        3
Impacted Files Coverage Δ
packages/jest-config/src/get_max_workers.js 100% <ø> (ø)
packages/jest-cli/src/run_jest.js 0% <ø> (ø) ⬆️
packages/jest-config/src/index.js 0% <ø> (ø) ⬆️
packages/jest-config/src/normalize.js 84.75% <100%> (+0.09%) ⬆️
packages/jest-cli/src/test_runner.js 28.46% <50%> (ø) ⬆️

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 d58e466...5a57c1a. Read the comment docs.

@aaronabramov aaronabramov merged commit dd6c5c4 into jestjs:master Jul 12, 2017
@aaronabramov aaronabramov deleted the max_workers_to_global_config branch July 12, 2017 18:10
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
@aaronabramov
add maxWorkers to globalConfig (jestjs#4005)
b01abde
@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 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thymikee thymikee thymikee left review comments

@cpojer cpojer cpojer approved these changes

Assignees
No one assigned
Labels
CLA Signed ✔️
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aaronabramov @cpojer @thymikee @codecov-io @facebook-github-bot