test: formatting skip messages for TAP parsing #2109
Conversation
|
Nice. LGTM. |
|
Perhaps we should inform the tests if they are being run under tap so this can be configured in the common test helpers file per test run output type? |
|
@Fishrock123 You mean like a common ffunction with the template string? How can we know if the tap is enabled? |
|
I think the text is as readable in tap output as in plain text. The test runner knows what mode we're in, but it feels like an unnecessary complication. We should really start abstracting output if we care about this. |
Eh, ok. LGTM |
|
LGTM. Maybe replace the process.exit calls with return statements while you're there, that should make reasoning about the test suite's remaining process.exit calls easier. |
|
@bnoordhuis Done! PTAL :-) |
|
@bnoordhuis I have never seen TAP in action. But looking at the Extended TAP Results in the CI, the changes didn't make any difference I guess. What am I missing? |
|
@thefourtheye you can see the output in the raw stdout logs. Not sure if jenkins registers skipped test as a part of the parsed output list. |
|
@jbergstroem Hmmm, in Console output also, I couldn't see any logs, but only the TAP results (ok/not ok) |
|
It looks like the test runner actually returns 'ok' on skip (well, return code 0). I'll have a look at this. This patch is still good to go though. |
thefourtheye
force-pushed
the
test-fixing-skip-messages
branch
from
d29f7d8
to
8a9922d
Compare
|
Okay, I rebased |
|
I'd suggest we keep the PR's separate. I probably have to update #2129 shortly (regex change) and neither PR relies on the other to work. No strong opinion about it though. |
|
@jbergstroem #2129 is not the |
|
@thefourtheye ah, I'll just continue studying the art of not reading properly. |
|
@jbergstroem lol, no problem :-) Do we need a separate CI run for this? |
|
@thefourtheye might as well; they're just idling. https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/126/ |
|
LGTM with a comment. |
thefourtheye
force-pushed
the
test-fixing-skip-messages
branch
from
8a9922d
to
555c226
Compare
| @@ -3,8 +3,8 @@ | |||
|
|
|||
| // FIXME add sunos support | |||
| if ('linux freebsd darwin'.indexOf(process.platform) === -1) { | |||
| console.error('Skipping test, platform not supported.'); | |||
| process.exit(); | |||
| console.log(`1..0 # Skipped: Unsupported platform [${process.platform}]`); | |||
There was a problem hiding this comment.
@bnoordhuis PTAL :-)
thefourtheye
force-pushed
the
test-fixing-skip-messages
branch
from
555c226
to
e8dec36
Compare
This patch makes the skip messages consistent so that the TAP plugin
in CI can parse the messages properly. The format will be
1..0 # Skipped: [Actual reason why the test is skipped]
This patch uses `return` statement to skip the test instead of using `process.exit` call.
|
@thefourtheye is this landable? |
|
@brendanashworth We can land this anytime but the relevant changes are in #2130. Cc @jbergstroem |
|
I'm LGTM for landing this. #2130 just makes it "visible". |
This patch makes the skip messages consistent so that the TAP plugin
in CI can parse the messages properly. The format will be
1..0 # Skipped: [Actual reason why the test is skipped]
PR-URL: #2109
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
This patch uses `return` statement to skip the test instead of using `process.exit` call. PR-URL: #2109 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>
This is a followup of nodejs#2109. The tests which didn't make it in nodejs#2109, are included in this patch. The skip messages are supposed to follow the format 1..0 # Skipped: [Actual reason why the test is skipped] and the tests should be skipped with the return statement.
This is a followup of #2109. The tests which didn't make it in #2109, are included in this patch. The skip messages are supposed to follow the format 1..0 # Skipped: [Actual reason why the test is skipped] and the tests should be skipped with the return statement. PR-URL: #2290 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Johan Bergström <[email protected]>
This patch makes the skip messages consistent so that the TAP plugin
in CI can parse the messages properly. The format will be
cc @bnoordhuis