test: TapReporter fixes

[skomski]

test: report timeout in TapReporter

Before: https://ci.nodejs.org/job/node-test-commit-arm/433/nodes=pi1-raspbian-wheezy-2_of_2/tapTestReport/test.tap-336/

[jbergstroem]

Is there a thing called # TIMEOUT? I reckon this should be "not ok" instead?

[rvagg]

no, this is wrong, it doesn't mean "duration in milliseconds", it means "duration with milliseconds"

[skomski]

@jbergstroem no it looks like this

1..1
# out/Release/node /home/skomski/Code/io.js/test/parallel/test-child-process-spawnsync-input.js
not ok 1 - test-child-process-spawnsync-input.js
# TIMEOUT
---
duration_s: 1.11
...
[1] 17638 exit 1 tools/test.py -p tap -t 1 -v parallel/test-child-process-spawnsync-input

[jbergstroem]

LGTM; it'll make reading a single test output easier (ref https://ci.nodejs.org/job/node-test-commit-arm/444/nodes=pi2-raspbian-wheezy-1_of_2/tapTestReport/test.tap-428/)

[thefourtheye]

Should we print it in the next line or on the same line as the status?

[jbergstroem]

@thefourtheye happy with both. I guess it'd be more consistent to put it on the same line, similar to how we output skip.

[skomski]

I put it on the next line because I thought it's different to #TODO or #SKIP in the sense that it's reason the test failed and not a simple comment to the test.

[jbergstroem]

@skomski what about # not ok - timeout

[jbergstroem]

ping @skomski

[skomski]

@jbergstroem why duplicate not ok or do you mean not ok 1 - test-child-process-spawnsync-input.js - timeout?

[jbergstroem]

@skomski the later.

[skomski]

@jbergstroem I did not look at the tap specification before:

http://testanything.org/tap-specification.html

ok 1 Description # Directive
# Diagnostic

The only defined directives are TODO and SKIP and it has no place in the description.
So I think the current place on the next line makes the most sense.

[skomski]

Merge?

[jbergstroem]

Hey, sorry. Fell out out of scope. Should play well with tap version 13. LGTM. It'd be good to get someone else's opinion as well; @thefourtheye?

[rvagg]

lgtm

[thefourtheye]

LGTM. Just tested it in my machine.

1..946
not ok 1 test-arm-math-exp-regress-1376.js
# TIMEOUT
---
duration_ms: 60.27
...
ok 2 test-assert.js
---
duration_ms: 0.265
...

[jbergstroem]

Cool. I'll merge.

[jbergstroem]

Merged in 10924ce. Closing