Skip to content

Use light test flag by default#19362

Merged
weswigham merged 2 commits into
microsoft:masterfrom
weswigham:light-tests-by-default
Oct 20, 2017
Merged

Use light test flag by default#19362
weswigham merged 2 commits into
microsoft:masterfrom
weswigham:light-tests-by-default

Conversation

@weswigham
Copy link
Copy Markdown
Member

@weswigham weswigham commented Oct 19, 2017

CI will still have the flag off, so we should still get the additional checks there; but this significantly reduces test runtime when running locally - assertInvariants dominates the runtime of many tests; especially those which include lib files, such as many jsx tests. Overall, this is ~25% savings in wall-clock time when running tests locally. This is likely fine since the invariant assertion rarely catches any bugs anymore, and it'll still be running on CI to catch things before they get merged.

Comment thread Jakefile.js Outdated
var testTimeout = process.env.timeout || defaultTestTimeout;
var tests = process.env.test || process.env.tests || process.env.t;
var light = process.env.light || false;
var light = process.env.light === undefined ? true : (process.env.light !== "false");
Copy link
Copy Markdown
Member

@DanielRosenwasser DanielRosenwasser Oct 20, 2017

Choose a reason for hiding this comment

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

What about process.env.light === undefined || process.env.light !== "false"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

probably fine.

@weswigham weswigham merged commit 3f406bd into microsoft:master Oct 20, 2017
@weswigham weswigham deleted the light-tests-by-default branch October 20, 2017 00:15
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants