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: use --port=0 in debugger tests that do not have to work on 9229 #44342

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung commented Aug 22, 2022

To avoid failures when there is another running process occupying
the port 9229 which may happen if there is a stale process, use the
--port argument of node-inspect to use a random port in tests that
don't have to work on port 9229.

The following tests are not touched:

  • test-debugger-launch: specifically needs to test port 9229
  • test-debugger-pid: needs modifications to node-inspect
  • test-debugger-random-port-with-inspect-port: same as -pid test

Refs: nodejs/build#3014

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 22, 2022
To avoid failures when there is another running process occupying
the port 9229 which may happen if there is a stale process, use the
--port argument of node-inspect to use a random port in tests that
don't have to work on port 9229.

The following tests are not touched:

- test-debugger-launch: specifically needs to test port 9229
- test-debugger-pid: needs modifications to node-inspect
- test-debugger-random-port-with-inspect-port: same as -pid test
@joyeecheung
Copy link
Member Author

joyeecheung commented Aug 22, 2022

The tests here can fail if one runs a node inspect process that listens to 9229 on the background before running them. As far as I can tell though, they don't actually need to be run on 9229, all they need are just a valid node-inspect session, so changing them to run on random ports seem reasonable to me and can help with nodejs/build#3014 which has been failing a lot of windows CI runs lately (see nodejs/reliability#354). test-debugger-pid and test-debugger-random-port-with-inspect-port would need some fixes in node-inspect to run on random ports, as node-inspect still waits for 9229 to free up when --inspect-port or -p is used, I'll send in another PR for those.

@joyeecheung
Copy link
Member Author

cc @nodejs/testing

@nodejs-github-bot
Copy link
Collaborator

@joyeecheung
Copy link
Member Author

hmm, apparently the tests are still timing out, so there must be something else that is causing the timeout

@joyeecheung
Copy link
Member Author

It's too old now. I'll open a new one and move them to parallel.

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. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants