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

tools: make --repeat work with -j in test.py #9249

Closed
wants to merge 1 commit into from
Closed

tools: make --repeat work with -j in test.py #9249

wants to merge 1 commit into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 24, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

tools test

Description of change

The repeat option in test.py did not work as expected if -j was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting thread_id on one of them
will now not change the thread_id on all of them. And thread_id is
how the temp directory (and common.PORT as well) are determined.

@santigimeno @mhdawson

@Trott Trott added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Oct 24, 2016
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label Oct 24, 2016
@Trott
tools: make --repeat work with -j in test.py
d60f00a 
The repeat option in test.py did not work as expected if `-j` was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

    tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting `thread_id` on one of them
will now not change the `thread_id` on all of them. And `thread_id` is
how the temp directory (and common.PORT as well) are determined.

Refs: nodejs#9228
@Trott
Copy link
Member Author

Trott commented Oct 24, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/4629/

@gibfahn
Copy link
Member

gibfahn commented Oct 24, 2016

I've definitely seen fs tests failing due to this error, thanks @Trott !

santigimeno
santigimeno approved these changes Oct 24, 2016
View reviewed changes
Copy link
Member

@santigimeno santigimeno left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

jasnell pushed a commit that referenced this pull request Oct 26, 2016
@Trott @jasnell
tools: make --repeat work with -j in test.py
60a78ae 
The repeat option in test.py did not work as expected if `-j` was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

    tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting `thread_id` on one of them
will now not change the `thread_id` on all of them. And `thread_id` is
how the temp directory (and common.PORT as well) are determined.

Refs: #9228
PR-URL: #9249
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 26, 2016

Landed in 60a78ae

@jasnell jasnell closed this Oct 26, 2016
evanlucas pushed a commit that referenced this pull request Nov 3, 2016
@Trott @evanlucas
tools: make --repeat work with -j in test.py
bdad1e2 
The repeat option in test.py did not work as expected if `-j` was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

    tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting `thread_id` on one of them
will now not change the `thread_id` on all of them. And `thread_id` is
how the temp directory (and common.PORT as well) are determined.

Refs: #9228
PR-URL: #9249
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@evanlucas evanlucas mentioned this pull request Nov 3, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
@Trott
tools: make --repeat work with -j in test.py
533ce48 
The repeat option in test.py did not work as expected if `-j` was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

    tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting `thread_id` on one of them
will now not change the `thread_id` on all of them. And `thread_id` is
how the temp directory (and common.PORT as well) are determined.

Refs: #9228
PR-URL: #9249
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
@Trott
tools: make --repeat work with -j in test.py
2388648 
The repeat option in test.py did not work as expected if `-j` was set to
more than one. Repeated tests running at the same time could share temp
directories and cause test failures. This was observed with:

    tools/test.py -J --repeat=10 parallel/test-fs-watch-recursive

By using copy.deepCopy(), the repeated tests are separate objects and
not references to the same objects. Setting `thread_id` on one of them
will now not change the `thread_id` on all of them. And `thread_id` is
how the temp directory (and common.PORT as well) are determined.

Refs: #9228
PR-URL: #9249
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
This was referenced Nov 22, 2016
Merged
Merged
@Trott Trott deleted the repeat branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gibfahn gibfahn gibfahn approved these changes

@mhdawson mhdawson mhdawson approved these changes

@santigimeno santigimeno santigimeno approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Trott @gibfahn @jasnell @thefourtheye @santigimeno @mhdawson @MylesBorins @nodejs-github-bot