test: fix flaky test-http-highwatermark #17949
Conversation
|
CI: https://ci.nodejs.org/job/node-test-pull-request/12371/ (1 failure unrelated) |
|
@apapirovski Would you be able to run a stress test on OSX for this, it seems like that was the platform it was failing on most often cc @Trott |
|
Sure. Here it is: https://ci.nodejs.org/job/node-stress-single-test/1596/ (Although I'm on OS X so I already ran it locally.) Edit: New one after making a minor tweak based on the feedback below: https://ci.nodejs.org/job/node-stress-single-test/1606/ |
| @@ -39,11 +39,11 @@ const server = http.createServer(function(req, res) { | |||
| }).on('listening', () => { | |||
| const c = net.createConnection(server.address().port, () => { | |||
| c.write('GET / HTTP/1.1\r\n\r\n'); | |||
| c.write('GET / HTTP/1.1\r\n\r\n'); | |||
| c.write('GET / HTTP/1.1\r\n\r\n', | |||
| () => setImmediate(() => c.on('data', () => {}))); | |||
There was a problem hiding this comment.
Nit: c.resume()? It removes the noop.
apapirovski
added
the
author ready
|
Another minimal CI to verify the fixup: https://ci.nodejs.org/job/node-test-commit-light/99/ |
PR-URL: nodejs#17949 Fixes: nodejs#17857 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
|
Landed in a51944d |
PR-URL: #17949 Fixes: #17857 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #17949 Fixes: #17857 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: #17949 Fixes: #17857 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
TimothyGu
removed
the
author ready
PR-URL: #17949 Fixes: #17857 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
|
@apapirovski @BridgeAR I have opened a PR for a different issue #30184 in which this test is the only one failing and I don't see an obvious reason why. I'd appreciate if you could take a look to see if there is smth wrong in my PR or the test #30184 |
The current version of the test is dependent on the requests coming in before the data is successfully sent & read. Make
writesize much larger and delay thereadon the other side a bit to get rid of the flakiness.Fixes: #17857
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
test