Skip to content

test: deflake connection refused proxy tests#63395

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-http-proxy-request-connection-refused
Open

test: deflake connection refused proxy tests#63395
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:flaky-test-http-proxy-request-connection-refused

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 17, 2026

This deflakes the HTTP and HTTPS connection-refused proxy tests.

The tests previously bound an ephemeral port, closed it, and then expected a
child process to get ECONNREFUSED when connecting to that released port. That
left a race where the port could be reused before the child process connected.

This updates the tests to use 127.0.0.1:10, matching existing
refused-connection tests, and clears NO_PROXY/no_proxy so local proxy
bypass settings do not skip the proxy connection attempt.

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-05-15.md#jstest-failure


Assisted-by: openai:gpt-5.5

Use 127.0.0.1:10, matching existing refused-connection tests, instead
of binding and releasing an ephemeral port that can be reused before
the child process connects.

Clear NO_PROXY and no_proxy so local proxy bypass settings do not skip
the proxy connection attempt.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@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 May 17, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.08%. Comparing base (1f371fc) to head (cd1aa1b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #63395      +/-   ##
==========================================
+ Coverage   90.05%   90.08%   +0.03%     
==========================================
  Files         714      714              
  Lines      225704   225704              
  Branches    42714    42735      +21     
==========================================
+ Hits       203250   203331      +81     
+ Misses      14225    14165      -60     
+ Partials     8229     8208      -21     

see 25 files with indirect coverage changes

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

I feel that there's still a risk that port 10 is used by some process, though that seems not super likely, in any case if that happens again I guess we can try probing a few more unlikely-to-be-used ports before using them for testing..

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 18, 2026
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. needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants