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: move test-process-title to sequential #33150

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that foo does not appear in test-process-title
but in test-process-title-cli):

not ok 1727 parallel/test-process-title
  ---
  duration_ms: 0.156
  severity: fail
  exitcode: 1
  stack: |-
    assert.js:103
      throw new AssertionError(obj);
      ^

    AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
    + actual - expected

    + 'foo'
    - 'd:\\a\\node\\node\\out\\Release\\node.exe'
        at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
        at Module._compile (internal/modules/cjs/loader.js:1176:30)
        at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
        at Module.load (internal/modules/cjs/loader.js:1040:32)
        at Function.Module._load (internal/modules/cjs/loader.js:929:14)
        at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
        at internal/main/run_main_module.js:17:47 {
      generatedMessage: true,
      code: 'ERR_ASSERTION',
      actual: 'foo',
      expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
      operator: 'strictEqual'
    }
  ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):

    not ok 1727 parallel/test-process-title
      ---
      duration_ms: 0.156
      severity: fail
      exitcode: 1
      stack: |-
        assert.js:103
          throw new AssertionError(obj);
          ^

        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + 'foo'
        - 'd:\\a\\node\\node\\out\\Release\\node.exe'
            at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
            at Module._compile (internal/modules/cjs/loader.js:1176:30)
            at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
            at Module.load (internal/modules/cjs/loader.js:1040:32)
            at Function.Module._load (internal/modules/cjs/loader.js:929:14)
            at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
            at internal/main/run_main_module.js:17:47 {
          generatedMessage: true,
          code: 'ERR_ASSERTION',
          actual: 'foo',
          expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
          operator: 'strictEqual'
        }
      ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 29, 2020
@addaleax
Copy link
Member Author

👍 to fast-track? Not sure how flaky this is exactly but this should be very low-risk.

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Apr 29, 2020
@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Should we move that too?

@addaleax
Copy link
Member Author

@richardlau I think moving one should be sufficient.

There are also other tests that set the process title, but I think this one is affected specifically because it spawns a new child process to get the current process title (i.e. there is a race condition window between the process.title = process.execPath line and the child process reading process.title). I don’t think the other tests are affected in the same way because they only access process.title inside the a single process, but I guess somebody with more Windows experience would have to confirm that.

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 29, 2020
@nodejs-github-bot
Copy link
Collaborator

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

puzpuzpuz pushed a commit that referenced this pull request Apr 30, 2020
This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):

    not ok 1727 parallel/test-process-title
      ---
      duration_ms: 0.156
      severity: fail
      exitcode: 1
      stack: |-
        assert.js:103
          throw new AssertionError(obj);
          ^

        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + 'foo'
        - 'd:\\a\\node\\node\\out\\Release\\node.exe'
            at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
            at Module._compile (internal/modules/cjs/loader.js:1176:30)
            at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
            at Module.load (internal/modules/cjs/loader.js:1040:32)
            at Function.Module._load (internal/modules/cjs/loader.js:929:14)
            at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
            at internal/main/run_main_module.js:17:47 {
          generatedMessage: true,
          code: 'ERR_ASSERTION',
          actual: 'foo',
          expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
          operator: 'strictEqual'
        }
      ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)

PR-URL: #33150
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@puzpuzpuz
Copy link
Member

Landed in ab41b98

@puzpuzpuz puzpuzpuz closed this Apr 30, 2020
targos pushed a commit that referenced this pull request May 4, 2020
This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):

    not ok 1727 parallel/test-process-title
      ---
      duration_ms: 0.156
      severity: fail
      exitcode: 1
      stack: |-
        assert.js:103
          throw new AssertionError(obj);
          ^

        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + 'foo'
        - 'd:\\a\\node\\node\\out\\Release\\node.exe'
            at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
            at Module._compile (internal/modules/cjs/loader.js:1176:30)
            at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
            at Module.load (internal/modules/cjs/loader.js:1040:32)
            at Function.Module._load (internal/modules/cjs/loader.js:929:14)
            at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
            at internal/main/run_main_module.js:17:47 {
          generatedMessage: true,
          code: 'ERR_ASSERTION',
          actual: 'foo',
          expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
          operator: 'strictEqual'
        }
      ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)

PR-URL: #33150
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
@targos targos mentioned this pull request May 4, 2020
targos pushed a commit that referenced this pull request May 7, 2020
This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):

    not ok 1727 parallel/test-process-title
      ---
      duration_ms: 0.156
      severity: fail
      exitcode: 1
      stack: |-
        assert.js:103
          throw new AssertionError(obj);
          ^

        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + 'foo'
        - 'd:\\a\\node\\node\\out\\Release\\node.exe'
            at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
            at Module._compile (internal/modules/cjs/loader.js:1176:30)
            at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
            at Module.load (internal/modules/cjs/loader.js:1040:32)
            at Function.Module._load (internal/modules/cjs/loader.js:929:14)
            at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
            at internal/main/run_main_module.js:17:47 {
          generatedMessage: true,
          code: 'ERR_ASSERTION',
          actual: 'foo',
          expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
          operator: 'strictEqual'
        }
      ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)

PR-URL: #33150
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
targos pushed a commit that referenced this pull request May 13, 2020
This test can fail when run in parallel with test-process-title-cli,
which also sets the process title, which is global state on Windows.

Example failure (note that `foo` does not appear in test-process-title
but in test-process-title-cli):

    not ok 1727 parallel/test-process-title
      ---
      duration_ms: 0.156
      severity: fail
      exitcode: 1
      stack: |-
        assert.js:103
          throw new AssertionError(obj);
          ^

        AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
        + actual - expected

        + 'foo'
        - 'd:\\a\\node\\node\\out\\Release\\node.exe'
            at Object.<anonymous> (d:\a\node\node\test\parallel\test-process-title.js:22:1)
            at Module._compile (internal/modules/cjs/loader.js:1176:30)
            at Object.Module._extensions..js (internal/modules/cjs/loader.js:1196:10)
            at Module.load (internal/modules/cjs/loader.js:1040:32)
            at Function.Module._load (internal/modules/cjs/loader.js:929:14)
            at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
            at internal/main/run_main_module.js:17:47 {
          generatedMessage: true,
          code: 'ERR_ASSERTION',
          actual: 'foo',
          expected: 'd:\\a\\node\\node\\out\\Release\\node.exe',
          operator: 'strictEqual'
        }
      ...

(from https://github.com/nodejs/node/runs/628144750?check_suite_focus=true)

PR-URL: #33150
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants