Update some more tests relying on timeouts (#341)

* Remove unnecessary timeout check in httpClient delay
In addition it's probably better to use a t.setTimeout(..) than assertion for a timeout because it will come with a clear and informative error message.

* Update a couple of tests that don't do well in low CPU env.

Avoid relying on node being able to execute its setTimeouts within a reasonable time to account for the low performance VMs of the tests environments.

* add empty line for readability in a test

* Allow sync call of httpClient.destroy() in a timeout event

lib/httpClient.js

@@ -64,15 +64,15 @@ function Client (opts) {
   this.opts.setupClient(this)
 
   const handleTimeout = () => {
-    // all pipelined requests have timed out here
-    this.resData.forEach(() => this.emit('timeout'))
     this.cer = 0
     this._destroyConnection()
 
-    // timeout has already occured, need to set a new timeoutTicker
-    this.timeoutTicker = retimer(handleTimeout, this.timeout)
+    this.timeoutTicker.reschedule(this.timeout)
 
     this._connect()
+
+    // all pipelined requests have timed out here
+    this.resData.forEach(() => this.emit('timeout'))
   }
 
   if (this.rate) {

package.json

@@ -65,7 +65,7 @@
     "pretty-bytes": "^5.4.1",
     "progress": "^2.0.3",
     "reinterval": "^1.1.0",
-    "retimer": "^2.0.0",
+    "retimer": "^3.0.0",
     "semver": "^7.3.2",
     "timestring": "^6.0.0"
   }

test/httpClient.test.js

@@ -581,9 +581,8 @@ test('client should emit a timeout when no response is received', (t) => {
 
   client.on('timeout', () => {
     t.ok(1, 'timeout should have happened')
+    client.destroy()
   })
-
-  setTimeout(() => client.destroy(), 1800)
 })
 
 test('client should emit 2 timeouts when no responses are received', (t) => {
@@ -592,12 +591,13 @@ test('client should emit 2 timeouts when no responses are received', (t) => {
   const opts = timeoutServer.address()
   opts.timeout = 1
   const client = new Client(opts)
-
+  let count = 0
   client.on('timeout', () => {
     t.ok(1, 'timeout should have happened')
+    if (count++ > 0) {
+      client.destroy()
+    }
   })
-
-  setTimeout(() => client.destroy(), 2800)
 })
 
 test('client should have 2 different requests it iterates over', (t) => {