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: fix inspector tests #16281

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Oct 18, 2017

The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions ("address already in use") will happen.

This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.

Example problem:

https://ci.nodejs.org/job/node-test-commit-linuxone/9482/nodes=rhel72-s390x/console

not ok 973 parallel/test-inspector-async-hook-teardown-at-debug-end
  ---
  duration_ms: 0.112
  severity: fail
  stack: |-
    Starting inspector on 127.0.0.1:9229 failed: address already in use
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

test inspector

@Trott Trott added flaky-test Issues and PRs related to the tests with unstable failures on the CI. inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests. labels Oct 18, 2017
The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions will happen.

This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.
@Trott
Copy link
Member Author

Trott commented Oct 18, 2017

@Trott
Copy link
Member Author

Trott commented Oct 18, 2017

@nodejs/collaborators If you happen to be available right about now, it would be great to get this reviewed and fast-tracked.

@Trott
Copy link
Member Author

Trott commented Oct 18, 2017

Single failure in Linux is unrelated. Failures on Raspberry Pi device is a build problem on a single device (that has since been taken offline). Everything else passed.

@Trott
Copy link
Member Author

Trott commented Oct 18, 2017

Given the high impact on CI and the ease with which any of this can be reverted if necessary (it's mostly moving files from one directory to another), I'm going to land...

Trott added a commit to Trott/io.js that referenced this pull request Oct 18, 2017
The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions will happen.

This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.

PR-URL: nodejs#16281
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
@Trott
Copy link
Member Author

Trott commented Oct 18, 2017

Landed in 9be3d99

@Trott Trott closed this Oct 18, 2017
targos pushed a commit that referenced this pull request Oct 18, 2017
The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions will happen.

This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.

PR-URL: #16281
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 18, 2017
The inspector tests should not be in the parallel directory as they
likely all (or certainly almost all) use static ports, so port
collisions will happen.

This moves them all to sequential. We can move them back on a
case-by-case basis. They were run sequentially when they were in the
inspector directory which they were only moved from very recently.

PR-URL: nodejs/node#16281
Reviewed-By: Daniel Bevenius <[email protected]>
Reviewed-By: Yuta Hiroto <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Bryan English <[email protected]>
@refack refack mentioned this pull request Oct 18, 2017
4 tasks
@maclover7
Copy link
Contributor

sorry this was my bad 😞

@Trott Trott deleted the fix-inspector-tirefire branch January 13, 2022 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. inspector Issues and PRs related to the V8 inspector protocol test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants