Investigate flaky test test-http-client-timeout-event

[joaocgreis]

Examples of failures:

• pi1-raspbian-wheezy

[brendanashworth]

There seems to be a race condition between timers. I'll try to send in a PR in the coming week.

[Trott]

Possible solution might be to use common.platformTimeout() on those timeouts to put a little more space between them on the Pi 1 so that they run in the expected order.

[brendanashworth]

@Trott good tip, thanks. I ended up doing the platform timeout for part of it, but refractored some of it to allow for ending early and reducing racy-ness. Here's the diff if you want it, I'll submit the PR once I get access to a pi to double check:

diff --git a/test/parallel/test-http-client-timeout-event.js b/test/parallel/test-http-client-timeout-event.js
index c9d6594..77776eb 100644
--- a/test/parallel/test-http-client-timeout-event.js
+++ b/test/parallel/test-http-client-timeout-event.js
@@ -25,16 +25,24 @@ server.listen(options.port, options.host, function() {
     server.close();
   });

-  var timeout_events = 0;
   req.setTimeout(1);
-  req.on('timeout', function() {
-    timeout_events += 1;
-  });
-  setTimeout(function() {
+  req.on('timeout', common.mustCall(function() {
+    clearTimeout(timer);
+
     req.destroy();
-    assert.equal(timeout_events, 1);
-  }, 100);
-  setTimeout(function() {
-    req.end();
-  }, 50);
+  }, 1));
+
+  // Emit the request immediately, and once the request is given a socket,
+  // ensure that the timeout has been emitted after ~100ms. If the timeout
+  // is called before that, the test will exit early.
+  req.end();
+
+  var timer;
+  req.on('socket', function() {
+    // If the timer is called, then the timeout is either taking an
+    // extraordinarily long time, or it won't fire.
+    timer = setTimeout(function() {
+      throw new Error('timeout was not called');
+    }, common.platformTimeout(100));
+  });
 });

[brendanashworth]

I wasn't able to see any flakiness before or after on a raspberry pi after 1000 runs each, so the PR may just be in the blind.

cc @rvagg I'm done with the machine, thanks!

[Trott]

@brendanashworth Stress test on current master shows that this test is still flaky: https://ci.nodejs.org/job/node-stress-single-test/24/nodes=pi1-raspbian-wheezy/console

Fails with:

assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: 0 == 1
    at null._onTimeout (/home/iojs/build/workspace/node-stress-single-test/nodes/pi1-raspbian-wheezy/test/parallel/test-http-client-timeout-event.js:35:12)
    at Timer.listOnTimeout (timers.js:92:15)

If you've got a publicly available branch on GitHub with your proposed solution, we can run that through the stress test and see if it fails or not.

[Trott]

Just subbing in common.platform() got rid of the flakiness. 3421 runs without a failure: https://ci.nodejs.org/job/node-stress-single-test/nodes=pi1-raspbian-wheezy/27/console It was set for 9999 runs, but that would have taken over 11 hours (at the rate of 4 seconds per test on Raspberry Pi) so it looks like Rod manually killed it at that point.

@brendanashworth I'll submit a PR with just the common.platform() changes. If you'd like to submit your changes here instead, I'll close that PR.

[Trott]

#3968