Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tap gets confused with multiple tests providing timeout #23

Closed
trentm opened this issue Dec 5, 2011 · 1 comment
Closed

tap gets confused with multiple tests providing timeout #23

trentm opened this issue Dec 5, 2011 · 1 comment

Comments

@trentm
Copy link
Contributor

trentm commented Dec 5, 2011

First case: one test of four with a 'timeout' setting. Another test has a bogus config object ('xxx' key). This suite as expected.

var test = require('tap').test;
console.warn("[%s] Start.", new Date())
test("one", function(t) {
  t.ok("one", "one potato");
  t.end();
});
test("two", {timeout: 3000}, function(t) {
  setTimeout(function() {
    t.ok("two", "two potato");
    t.end()
    console.warn("[%s] Finished executing test two.", new Date())
  }, 7000);
});
test("three", {xxx: 1000}, function(t) {
  t.ok("three", "three potato");
  t.end();
});
test("four", function(t) {
  t.ok("four", "four");
  t.end();
});

output:

$ node foo.js 
[Mon Dec 05 2011 11:24:11 GMT-0800 (PST)] Start.
# one
ok 1 one potato
# two
not ok 2 Timeout!
  ---
    file:   timers.js
    line:   83
    column: 39
    stack:  
      - getCaller (/Users/trentm/src/node-tap/lib/tap-assert.js:376:17)
      - assert (/Users/trentm/src/node-tap/lib/tap-assert.js:17:16)
      - Function.fail (/Users/trentm/src/node-tap/lib/tap-assert.js:91:10)
      - Test._testAssert [as fail] (/Users/trentm/src/node-tap/lib/tap-test.js:86:16)
      - Test.timeout (/Users/trentm/src/node-tap/lib/tap-test.js:30:8)
      - Object._onTimeout (native)
      - Timer.callback (timers.js:83:39)
  ...
# three
ok 3 three potato
# four
ok 4 four

1..4
# tests 4
# pass  3
# fail  1
[Mon Dec 05 2011 11:24:18 GMT-0800 (PST)] Finished executing test two.

Second case: Both tests "two" and "three" have a timeout. We expect test two to timeout (as above) and test "three" to complete within its timeout:

var test = require('tap').test;
console.warn("[%s] Start.", new Date())
test("one", function(t) {
  t.ok("one", "one potato");
  t.end();
});
test("two", {timeout: 3000}, function(t) {
  setTimeout(function() {
    t.ok("two", "two potato");
    t.end()
    console.warn("[%s] Finished executing test two.", new Date())
  }, 7000);
});
test("three", {timeout: 1000}, function(t) {
  t.ok("three", "three potato");
  t.end();
});
test("four", function(t) {
  t.ok("four", "four");
  t.end();
});

output:

$ node foo.js 
[Mon Dec 05 2011 11:24:58 GMT-0800 (PST)] Start.
# one
ok 1 one potato
# two
not ok 2 Timeout!
  ---
    file:   timers.js
    line:   83
    column: 39
    stack:  
      - getCaller (/Users/trentm/src/node-tap/lib/tap-assert.js:376:17)
      - assert (/Users/trentm/src/node-tap/lib/tap-assert.js:17:16)
      - Function.fail (/Users/trentm/src/node-tap/lib/tap-assert.js:91:10)
      - Test._testAssert [as fail] (/Users/trentm/src/node-tap/lib/tap-test.js:86:16)
      - Test.timeout (/Users/trentm/src/node-tap/lib/tap-test.js:30:8)
      - Object._onTimeout (native)
      - Timer.callback (timers.js:83:39)
  ...
# three
not ok 3 Timeout!
  ---
    file:   timers.js
    line:   83
    column: 39
    stack:  
      - getCaller (/Users/trentm/src/node-tap/lib/tap-assert.js:376:17)
      - assert (/Users/trentm/src/node-tap/lib/tap-assert.js:17:16)
      - Function.fail (/Users/trentm/src/node-tap/lib/tap-assert.js:91:10)
      - Test._testAssert [as fail] (/Users/trentm/src/node-tap/lib/tap-test.js:86:16)
      - Test.timeout (/Users/trentm/src/node-tap/lib/tap-test.js:30:8)
      - Object._onTimeout (native)
      - Timer.callback (timers.js:83:39)
  ...
ok 4 three potato
not ok 5 end called more than once
  ---
    file:   /Users/trentm/src/node-tap/tmp/foo.js
    line:   33
    column: 5
    stack:  
      - getCaller (/Users/trentm/src/node-tap/lib/tap-assert.js:376:17)
      - assert (/Users/trentm/src/node-tap/lib/tap-assert.js:17:16)
      - Function.fail (/Users/trentm/src/node-tap/lib/tap-assert.js:91:10)
      - Test.end (/Users/trentm/src/node-tap/lib/tap-harness.js:102:29)
      - Test.<anonymous> (/Users/trentm/src/node-tap/tmp/foo.js:33:5)
      - Test.<anonymous> (native)
      - Test.<anonymous> (events.js:61:17)
      - Test.emit (/Users/trentm/src/node-tap/lib/tap-test.js:102:8)
      - GlobalHarness.<anonymous> (/Users/trentm/src/node-tap/lib/tap-harness.js:86:13)
      - Array.0 (native)
  ...
# four
ok 6 four

1..6
# tests 6
# pass  3
# fail  3
[Mon Dec 05 2011 11:25:05 GMT-0800 (PST)] Finished executing test two.

IOW, this does NOT work as expected.

On a possibly related note, you can see from the printed timestamps that the test run does not terminate until the 7s "setTimeout" still runs to completion. Say this is a test case that takes a crazy long time, or forever to complete. Ideally I'd want my run to be aborted after the timeout, but I don't know how I'd implement that in this case.

Third case: Extend the second timeout (for test "three") to 13s so that there is sufficient time after test "two" has timed out and completed as well. Code:

var test = require('tap').test;
console.warn("[%s] Start.", new Date())
test("one", function(t) {
  t.ok("one", "one potato");
  t.end();
});
test("two", {timeout: 3000}, function(t) {
  setTimeout(function() {
    t.ok("two", "two potato");
    t.end()
    console.warn("[%s] Finished executing test two.", new Date())
  }, 7000);
});
test("three", {timeout: 13000}, function(t) {
  t.ok("three", "three potato");
  t.end();
});
test("four", function(t) {
  t.ok("four", "four");
  t.end();
});

Result:

$ TAP=1 ./node_modules/.bin/tap foo.js
# one
ok 1 one potato
# two
not ok 2 Timeout!
# three
ok 3 three potato
# four
ok 4 four
# tests 4
# pass  3
# fail  1
ok 5 foo.js


1..5
# tests 5
# pass  4
# fail  1

I.e., this is as expected.

Theory: The timeout handling for each test case isn't correctly starting the count from when that test case itself is being run... but is starting from the start of the full test suite run (or rather the start of when this test file is being run if there are multiple test files being run).

@trentm
Copy link
Contributor Author

trentm commented Dec 5, 2011

Here is a fix: https://gist.github.com/1435149
I.e. only start the t.timeout setTimeout when the test starts running (i.e. at its "ready" event).

Isaac: does this look reasonable?

trentm added a commit to trentm/node-tap that referenced this issue Dec 7, 2011
@isaacs isaacs closed this as completed in 21603a4 Dec 7, 2011
isaacs added a commit that referenced this issue Sep 1, 2023
Fix #25
Fix #23

This moves around some things to make the following changes:

1. Stricter ordering of events.  Now a bailout will push a pending
assert event, rather than omitting it.

2. `Subtest` comments are now no longer semantically relevant.  They`re
just comments.  What makes a child test is having indented non-comment
TAP data.

3. Support for buffered subtests is re-added, and made a bit more
robust.

4. Invalid TAP that causes the parser to end in failure now also adds an
entry in the `failures` array, so it's possible to see what went wrong.

This is a breaking change, because of the semantic changes, but it
should be an easy upgrade for node-tap, its primary consumer.  There's a
possibility that it'll break some of the reporting, though, so it's
still very provisional.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants