From 49f1c03c07034f97ae42694e72edd3b51150fd13 Mon Sep 17 00:00:00 2001 From: isaacs Date: Mon, 24 Jun 2019 22:14:18 -0700 Subject: [PATCH] Fix timeout-update test on node v10 and v8 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. --- lib/base.js | 7 +++-- lib/spawn.js | 2 +- lib/tap.js | 6 +++++ tap-snapshots/test-spawn.js-TAP.test.js | 9 +++++++ .../test-tap-set-timeout.js-TAP.test.js | 26 +++++++++++++++++++ tap-snapshots/test-test.js-TAP.test.js | 6 ----- test/spawn.js | 23 +++++++++++----- test/tap/set-timeout.js | 4 +++ 8 files changed, 66 insertions(+), 17 deletions(-) create mode 100644 tap-snapshots/test-tap-set-timeout.js-TAP.test.js create mode 100644 test/tap/set-timeout.js diff --git a/lib/base.js b/lib/base.js index 508d055f3..630fe1a6b 100644 --- a/lib/base.js +++ b/lib/base.js @@ -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() @@ -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() + } } } diff --git a/lib/spawn.js b/lib/spawn.js index 0ec8a6c45..71b3624c9 100644 --- a/lib/spawn.js +++ b/lib/spawn.js @@ -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) diff --git a/lib/tap.js b/lib/tap.js index 4e03c4bd5..7a353818e 100644 --- a/lib/tap.js +++ b/lib/tap.js @@ -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) diff --git a/tap-snapshots/test-spawn.js-TAP.test.js b/tap-snapshots/test-spawn.js-TAP.test.js index 8c9175f98..36c08a354 100644 --- a/tap-snapshots/test-spawn.js-TAP.test.js +++ b/tap-snapshots/test-spawn.js-TAP.test.js @@ -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 diff --git a/tap-snapshots/test-tap-set-timeout.js-TAP.test.js b/tap-snapshots/test-tap-set-timeout.js-TAP.test.js new file mode 100644 index 000000000..d819ee6e8 --- /dev/null +++ b/tap-snapshots/test-tap-set-timeout.js-TAP.test.js @@ -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} + +` diff --git a/tap-snapshots/test-test.js-TAP.test.js b/tap-snapshots/test-test.js-TAP.test.js index 02d2b02c7..517e3caef 100644 --- a/tap-snapshots/test-test.js-TAP.test.js +++ b/tap-snapshots/test-test.js-TAP.test.js @@ -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 @@ -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! --- @@ -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! --- @@ -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 @@ -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! @@ -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 diff --git a/test/spawn.js b/test/spawn.js index 1e4e3839f..993851b39 100644 --- a/test/spawn.js +++ b/test/spawn.js @@ -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' ])) @@ -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({ @@ -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': diff --git a/test/tap/set-timeout.js b/test/tap/set-timeout.js new file mode 100644 index 000000000..2b646958e --- /dev/null +++ b/test/tap/set-timeout.js @@ -0,0 +1,4 @@ +require('./')(t => { + t.setTimeout(1234) + t.pass('fine') +})