8000 tools: make --repeat work with -j in test.py by Trott · Pull Request #9249 · nodejs/node · GitHub
  • [go: up one dir, main page]
    Skip to content

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

    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
    Closed
    Trott wants to merge 1 commit into from
    Closed

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

    Trott wants to merge 1 commit into from
    +4 −1

    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

    @jasnell jasnell jasnell approved these changes

    @santigimeno santigimeno santigimeno approved these changes

    @mhdawson mhdawson mhdawson approved these changes

    +2 more reviewers

    @thefourtheye thefourtheye thefourtheye approved these changes

    @gibfahn gibfahn gibfahn approved these changes

    Reviewers whose approvals may not affect merge requirements

    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
    0