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 flaky timeout-request body and headers tests #38045

Closed

Conversation

Linkgoron
Copy link
Member

@Linkgoron Linkgoron commented Apr 2, 2021

fix the flaky test-http-server-request-timeout-delayed-body and test-http-server-request-timeout-delayed-headers tests which sometimes fail on slow systems. This happened to me today on the raspberry pi CI, and I could reproduce this locally by running /tools/test.py -j250 --repeat=1000 test/parallel/test-http-server-request-timeout-delayed-body.js. I could not reproduce the issue after my fix.

The reason, I believe, is that sometimes it takes more than a second for the request to "reach" the server. I've changed the tests to delay the body/headers sending to only happen when we're sure that a connection was made with the server.

EDIT:
Also added the same fix to:

  • test/parallel/test-http-server-request-timeout-interrupted-body.js
  • test/parallel/test-http-server-request-timeout-interrupted-headers.js
  • test/parallel/test-http-server-request-timeout-upgrade.js

@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 Apr 2, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

targos commented Apr 3, 2021

@nodejs/testing

@Linkgoron Linkgoron added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Apr 3, 2021
@Linkgoron Linkgoron changed the title test: fix flaky timeout-delayed-body and headers tests test: fix flaky timeout-request body and headers tests Apr 3, 2021
@Linkgoron
Copy link
Member Author

Linkgoron commented Apr 3, 2021

Added the same fix to 3 other tests that check requestTimeout. There's also an open issue for flakiness in test-http-request-timeout-keepalive (which I didn't change), but I think that the flakiness there is caused by a different problem.

@Linkgoron Linkgoron force-pushed the test-delayed-timeout-flaky-tests branch from adf4624 to 3b82eee Compare April 3, 2021 16:50
@nodejs-github-bot
Copy link
Collaborator

@Linkgoron Linkgoron added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 5, 2021
fix the flaky test-http-server-request-timeout-delayed-body
and test-http-server-request-timeout-delayed-headers which
sometimes fail on slow systems.

PR-URL: nodejs#38045
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@Trott Trott force-pushed the test-delayed-timeout-flaky-tests branch from 3b82eee to d75543d Compare April 5, 2021 14:49
@Trott
Copy link
Member

Trott commented Apr 5, 2021

Landed in d75543d

@Trott Trott closed this Apr 5, 2021
targos pushed a commit that referenced this pull request May 1, 2021
fix the flaky test-http-server-request-timeout-delayed-body
and test-http-server-request-timeout-delayed-headers which
sometimes fail on slow systems.

PR-URL: #38045
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
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. 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.

5 participants