Skip to content

Commit

Permalink
Fix timeout-update test on node v10 and v8
Browse files Browse the repository at this point in the history
Also, only send the magic timer comment from the TAP root object, so we
don't litter the output with lots of #timeout=n comments in normal test
scenarios.
  • Loading branch information
isaacs committed Jun 25, 2019
1 parent c6b493f commit 49f1c03
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 17 deletions.
7 changes: 3 additions & 4 deletions lib/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,6 @@ class Base extends MiniPass {
}

setTimeout (n, quiet) {
if (n && typeof n === 'number' && !quiet)
this.write(`# timeout=${n}\n`)

if (!this.hrtime)
this.hrtime = hrtime()

Expand All @@ -141,8 +138,10 @@ class Base extends MiniPass {

this.timer = setTimeout(() => this.timeout(), n)
/* istanbul ignore else */
if (this.timer.unref)
if (this.timer.unref) {
this.timer.duration = n
this.timer.unref()
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Spawn extends Base {

main (cb) {
this.cb = cb
this.setTimeout(this.options.timeout, true)
this.setTimeout(this.options.timeout)
this.parser.on('comment', c => {
const tomatch = c.match(/# timeout=([0-9]+)\n$/)
if (tomatch)
Expand Down
6 changes: 6 additions & 0 deletions lib/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ class TAP extends Test {
this.resume = Test.prototype.resume
}

setTimeout (n, quiet) {
if (n && typeof n === 'number' && !quiet)
this.write(`# timeout=${n}\n`)
return super.setTimeout(n)
}

pipe () {
this[_unmagicPipe]()
const ret = this.pipe.apply(this, arguments)
Expand Down
9 changes: 9 additions & 0 deletions tap-snapshots/test-spawn.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ not ok 1 - timeout!
`

exports[`test/spawn.js TAP timeout update > must match snapshot 1`] = `
TAP version 13
# timeout=42069
ok 1 - this is fine
1..1
# {time}
`

exports[`test/spawn.js TAP using proc event > must match snapshot 1`] = `
TAP version 13
ok
Expand Down
26 changes: 26 additions & 0 deletions tap-snapshots/test-tap-set-timeout.js-TAP.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* IMPORTANT
* This snapshot file is auto-generated, but designed for humans.
* It should be checked into source control and tracked carefully.
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/tap/set-timeout.js TAP > exit status 1`] = `
Object {
"code": 0,
"signal": null,
}
`

exports[`test/tap/set-timeout.js TAP > stderr 1`] = `
`

exports[`test/tap/set-timeout.js TAP > stdout 1`] = `
TAP version 13
# timeout=1234
ok 1 - fine
1..1
# {time}
`
6 changes: 0 additions & 6 deletions tap-snapshots/test-test.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,6 @@ ok 16 - todo # TODO

exports[`test/test.js TAP assertions and weird stuff timeout at the last tick > output 1`] = `
TAP version 13
# timeout=1
# Subtest: work it harder buf=false
1..1
ok 1 - this is fine
Expand All @@ -1495,7 +1494,6 @@ ok 3 - work it harder buf=true # {time} {
1..1
ok 1 - this is fine
}
# timeout=1
not ok 4 - timeout!
---
Expand All @@ -1513,7 +1511,6 @@ not ok 4 - timeout!

exports[`test/test.js TAP assertions and weird stuff timeout expiration > output 1`] = `
TAP version 13
# timeout=50
# Subtest: get lost buf=false
not ok 1 - timeout!
---
Expand Down Expand Up @@ -1548,7 +1545,6 @@ not ok 2 - get lost buf=true # {time}
1..1
# failed 1 test
}
# timeout=50
1..2
# failed 2 of 2 tests
Expand All @@ -1557,7 +1553,6 @@ not ok 2 - get lost buf=true # {time}

exports[`test/test.js TAP assertions and weird stuff timeout with subs > output 1`] = `
TAP version 13
# timeout=50
# Subtest: get lost buf=false
# Subtest: carry on
not ok 1 - timeout!
Expand Down Expand Up @@ -1600,7 +1595,6 @@ not ok 2 - get lost buf=true # {time}
1..1
# failed 1 test
}
# timeout=50
1..2
# failed 2 of 2 tests
Expand Down
23 changes: 17 additions & 6 deletions test/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ process.env.TAP_BUFFER = ''
t.cleanSnapshot = require('./clean-stacks.js')

const main = () => {
// this test can be slow, especially on CI
t.setTimeout(120000)

t.test('basic child process', t =>
t.spawn(node, [ file, 'ok' ]))

Expand All @@ -31,10 +34,18 @@ const main = () => {
timeout: 1
}))

t.test('timeout update', t =>
t.spawn(node, [ file, 'timeout-update' ], {
timeout: process.env.CI ? 5000 : 500,
}))
t.test('timeout update', t => {
const s = new Spawn({
command: node,
args: [file, 'timeout-update'],
buffered: true,
})
t.plan(2)
s.main(() => {
t.equal(s.timer.duration, 42069, 'timer updated')
t.matchSnapshot(s.output)
})
})

t.test('timeout KILL', t => {
const s = new Spawn({
Expand Down Expand Up @@ -228,8 +239,8 @@ switch (process.argv[2]) {
break

case 'timeout-update':
t.setTimeout(10000)
setTimeout(() => t.pass('this is fine'), 500)
t.setTimeout(42069)
t.pass('this is fine')
break

case 'childId':
Expand Down
4 changes: 4 additions & 0 deletions test/tap/set-timeout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
require('./')(t => {
t.setTimeout(1234)
t.pass('fine')
})

0 comments on commit 49f1c03

Please sign in to comment.