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: remove test-http-exit-delay #4786

Closed
wants to merge 1 commit into from

Commits on Jan 21, 2016

  1. test: remove test-http-exit-delay

    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: nodejs#4277
    Trott committed Jan 21, 2016
    Configuration menu
    Copy the full SHA
    0731eb6 View commit details
    Browse the repository at this point in the history