mirror of https://github.com/lukechilds/node.git
Browse Source
test-http-exit-delay has a flaky history. Examination of the bug it was written to find suggests that the test should be removed. * The test is trying to find a delay of up to 1 second, but the delay may also be much smaller than that. So the test will not catch the bug all the time. It is therefore flaky when the bug exists. * Experience has shown that the test is flaky as well when the bug is absent in the code because it can sometimes take slower devices (such as the Raspberry Pi devices that we have in continuous integration) more than the allowed one second to run. Increasing the timeout for those devices will make the test pass but will also mean that the test isn't really testing anything because the delay it is trying to catch was a delay of up to one second. I don't think this is an appropriate test to run once on CI. If there is to be a test for the issue in question, it should be a benchmark test that is run a large number of times. We don't really have such tests in CI yet I would argue that this test is actively a problem. It does not reliably catch the issue it is supposed to catch, nor can it likely be made to do so. (To do so would likely require converting it to a benchmarking test as previously described. We don't run those in CI, at least not at this time.) Because this test will have both false positives and false negatives, especially on the slower devices, it contributes to a culture of dismissing failed tests. It does not reliably identify an issue nor does it reliably pass on a working code base. This test should be removed. Ref: https://github.com/nodejs/node/pull/4277 PR-URL: https://github.com/nodejs/node/pull/4786 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net>v4.x
Rich Trott
9 years ago
committed by
Myles Borins
1 changed files with 0 additions and 30 deletions
@ -1,30 +0,0 @@ |
|||||
'use strict'; |
|
||||
const assert = require('assert'); |
|
||||
const common = require('../common'); |
|
||||
const http = require('http'); |
|
||||
|
|
||||
var start; |
|
||||
const server = http.createServer(common.mustCall(function(req, res) { |
|
||||
req.resume(); |
|
||||
req.on('end', function() { |
|
||||
res.end('Success'); |
|
||||
}); |
|
||||
|
|
||||
server.close(); |
|
||||
})); |
|
||||
|
|
||||
server.listen(common.PORT, 'localhost', common.mustCall(function() { |
|
||||
start = new Date(); |
|
||||
const req = http.request({ |
|
||||
'host': 'localhost', |
|
||||
'port': common.PORT, |
|
||||
'agent': false, |
|
||||
'method': 'PUT' |
|
||||
}); |
|
||||
req.end('Test'); |
|
||||
})); |
|
||||
|
|
||||
process.on('exit', function() { |
|
||||
const end = new Date(); |
|
||||
assert(end - start < 1000, 'Entire test should take less than one second'); |
|
||||
}); |
|
Loading…
Reference in new issue