test: fix flaky http-end-throw-socket-handling

[santigimeno]

This test was sometimes failing because the internal timeout was fired
before the test completed. The reason was that sometimes some of the
connections were taking longer to be established. Removing the timeout
seems to fix the issue.

The trace I was getting on OS X was this:

=== release test-http-end-throw-socket-handling ===                     
Path: parallel/test-http-end-throw-socket-handling
/Users/sgimeno/node/node/test/parallel/test-http-end-throw-socket-handling.js:35
  throw new Error('Taking too long!');
  ^

Error: Taking too long!
    at null._onTimeout (/Users/sgimeno/node/node/test/parallel/test-http-end-throw-socket-handling.js:35:9)
    at Timer.unrefdHandle (timers.js:321:14)

The issue looks similar to the one that I observed on test-net-socket-timeout-unref: 538813c. Sometimes, when running the test suite, some TCP connections take a long time to be established (up to 5 seconds sometimes) (At least on my OS X box)

[jasnell]

/cc @Trott ... hmmm, I have to presume that the timeout was added intentionally. Perhaps a better approach would be to extend the length of the timeout as opposed to removing it?

[jasnell]

Since you're in this file making changes anyway, the require statements can be changed to use const

[Trott]

@jasnell wrote:

hmmm, I have to presume that the timeout was added intentionally. Perhaps a better approach would be to extend the length of the timeout as opposed to removing it?

Personally, I favor eliminating arbitrary timers like this one in tests unless the tests are benchmark/performance tests. But there is not universal agreement on this approach, unfortunately.

The benefit of the timer is this: If a situation arises where the test would fail in a way that it does not exit, then running it without the Python test wrapper will result in it not exiting and running it with the Python test wrapper will result in a timeout after a platform-specific timeout that is usually 60 seconds. In contrast, with the timer, instead of hanging, the test exits after a brief time with a friendly error message.

However, as we've seen again and again and again and again, with arbitrary timers like this, tests become flaky.

One solution is to increase the timeout value. Another is to get rid of the arbitrary timer altogether. I'm OK with either approach in this instance, but have a slight preference for removing the timer.

[jasnell]

CI: https://ci.nodejs.org/job/node-test-pull-request/2003/

[santigimeno]

This is superseeded by #5676 (I forgot I had sent this one previously)