-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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: remove timer from test-http-1.0 #5129
Conversation
Can we change wrap the callback function in |
I think one thing the |
LGTM if CI is green |
It's possible that the `end` event is emitted after the timeout fires causing the test to fail. Just remove the timer. If for some reason the `end` would never fire, the test will fail with a timeout.
5233710
to
d77f305
Compare
PR update with @Trott suggestion. |
@Trott Do you think I should close the PR then? Or can we try how it goes without the timer? |
@santigimeno Would a decent middle ground be to leave the (Or maybe we can confirm that the test runner on CI will close the server if the test leaves it open? Maybe create a test that runs before all others that opens a server on |
@Trott Sorry for the delay answering this, I've been very busy. |
Build bot failure on Windows in CI, running again: https://ci.nodejs.org/job/node-test-pull-request/1685/ |
@Trott ... any further issues with this? |
@jasnell No issues from me. LGTM |
It's possible that the `end` event is emitted after the timeout fires causing the test to fail. Just remove the timer. If for some reason the `end` would never fire, the test will fail with a timeout. PR-URL: #5129 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Landed in 6a3d38f |
It's possible that the `end` event is emitted after the timeout fires causing the test to fail. Just remove the timer. If for some reason the `end` would never fire, the test will fail with a timeout. PR-URL: #5129 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
It's possible that the `end` event is emitted after the timeout fires causing the test to fail. Just remove the timer. If for some reason the `end` would never fire, the test will fail with a timeout. PR-URL: #5129 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
It's possible that the `end` event is emitted after the timeout fires causing the test to fail. Just remove the timer. If for some reason the `end` would never fire, the test will fail with a timeout. PR-URL: #5129 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
It's possible that the
end
event is emitted after the timeout firescausing the test to fail. Just remove the timer. If for some reason the
end
would never fire, the test will fail with a timeout.It tries to fix the issue described here: #4600 (comment), that I also have rarely experienced on
Debian Jessie 64
.