Repository navigation
test: a small common.skip() improvement proposal #14016
Description
Activity
I think some test files are using it to partially skip tests, and let it print a skip message in the format that TAP understands; I’m not sure on the details, but you’d probably need to check the relevant files to see where there is actual code executed after the
skip().I think some test files are using it to partially skip tests, and let it print a skip message in the format that TAP understands; I’m not sure on the details, but you’d probably need to check the relevant files to see where there is actual code executed after the skip().
An example is test/parallel/test-buffer-alloc.js. (I'm actually not sure that isn't a code mistake TBH. I'm not sure TAP understands "partial skip". I think with TAP you either SKIP the test or you don't.)
It seems we have ~10 tests with partial skip now if I skim properly. So would it be OK to just add a message via
console.log()in these cases?@vsemozhetbyt Either that, or you could just rename the current
skipfunction to something else,common.printSkipMessageor whatever, then makeskipa wrapper around that +process.exit.Reacted by Vse Mozhe Buty@addaleax @Trott
I've found out one case whenreturn;is more appropriate thanprocess.exit(0): ifcommon.skip()is called after acommon.mustCall(), thenreturn;exits gracefully (with async callbacks fired) whileprocess.exit(0)causes error. There are 2 such files:parallel/test-dgram-bind-default-address.js
sequential/test-net-server-address.jsI've left them with
common.printSkipMessage()+return;.PR: #14021
- changed the title
[-]test: a smal common.skip() improvement proposal[/-][+]test: a small common.skip() improvement proposal[/+]on Jul 2, 2017 - added a commit that references this issue
on Jul 11, 2017 - added a commit that references this issue
on Jul 18, 2017 - added a commit that references this issue
on Jul 19, 2017 - added a commit that references this issue
on Sep 5, 2017
Currently,
common.skip()only outputs a message, so after each call, we need to addreturn;in tests — this is +~400 lines of code. Would it be safe to make this function likecommon.skipIfInspectorDisabled(), i.e. to addprocess.exit(0);incommon.skip()and remove all thereturn;in tests? If there are some +1 for this proposal, I can try to raise a PR.