test: use consistent timeouts#42893
Uh oh!
There was an error while loading. Please reload this page.
Conversation
nodejs-github-bot added needs-ci CC: @nodejs/http |
github-actionsbot removed the request-ci 
nodejs-github-bot commented Apr 28, 2022
nodejs-github-bot commented Apr 28, 2022
| @@ -0,0 +1,133 @@ | |||
| 'use strict' | |||
There was a problem hiding this comment.
Relying on timers is the most frequent source of flakiness in tests. Apart from doing it concurrently with multiple requests, does this test tests something not already tested in existing tests?
There was a problem hiding this comment.
I know, timers are messy. But unfortunately this test is about testing timers. :(
Yes, it checks that the concurrent checking of requestTimeout and headersTimeout works properly.
There was a problem hiding this comment.
My point is, are requestTimeout and headersTimeout already tested with a single request? If so what makes this test special? Why wouldn't the options work with multiple concurrent request if they work with a single one?
There was a problem hiding this comment.
Well, they should, but I thought adding a more complex "real world like" scenario would have helped for future regression testing.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Luigi Pinca <[email protected]>
ShogunPanda commented May 3, 2022
@lpinca Are we good on this PR? Can we land it? |
lpinca commented May 3, 2022
I have no objections. |
ShogunPanda commented May 3, 2022
@lpinca Do you mind leaving the second approval so I can proceed? |
nodejs-github-bot commented May 3, 2022
PR-URL: #42893 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Co-authored-by: Luigi Pinca <[email protected]>
ShogunPanda commented May 3, 2022
Landed in c3aa86d |
PR-URL: #42893 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Co-authored-by: Luigi Pinca <[email protected]>
juanarbol commented May 31, 2022
Sadly, a test seems to be break ( test/parallel/test-http-server-request-timeouts-mixed.js) in V16.x: $ ./node test/parallel/test-http-server-request-timeouts-mixed.js node:assert:123 throw new AssertionError(obj); ^ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: 60000 !== 2000 at Object.<anonymous> (/home/juanarbol/GitHub/node/test/parallel/test-http-server-request-timeouts-mixed.js:34:8) at Module._compile (node:internal/modules/cjs/loader:1105:14) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10) at Module.load (node:internal/modules/cjs/loader:981:32) at Function.Module._load (node:internal/modules/cjs/loader:822:12) at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12) at node:internal/main/run_main_module:17:47{generatedMessage: true, code: 'ERR_ASSERTION', actual: 60000, expected: 2000, operator: 'strictEqual'**}** |
7 participants
This PR enforces consistent timeouts for HTTP tests.