Fix infinite wait when invoking "bats -j5" instead of "bats -j 5" #657
Conversation
bats's option parser translates "-j5" to "-j -5". This caused a negative number of "slots" (threads) to be available and the test to silently hang forever. Not nice. Add a $num_jobs sanity check to fail immediately like this: ``` Invalid number of jobs: -5 tests.bats bats warning: Executed 0 instead of expected 3 tests 3 tests, 0 failures, 3 not run ``` Non-numeric arguments (e.g.: -jf) evaluate to zero which also fails the check. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
|
Thanks fior you report and suggested fix. This is a problem with the short flag unpacker and will cause trouble with other value arguments like -f as well. I believe we should also fix this for good. I can take care of that. Meanwhile, can you please add a test for your fix? |
Makes sense, indeed I thought about looking at the options parser. Then I remembered option parsers are always a complicated and ambiguous mess no matter how hard you try whereas I found this quick sanity check was dead simple to add (once I root-caused the issue). Also, I think a little bit of "defensive programming" is always good when infinite loops are involved!
Good! :-)
Depends how long it would take, BATS is completely new to me and I don't have a lot of spare time... Any starting point, similar test you can point me at? |
martin-schulze-vireso
force-pushed
the
fix-infinite-j
branch
from
d7c504a
to
f834cfc
Compare
martin-schulze-vireso
force-pushed
the
fix-infinite-j
branch
from
f834cfc
to
58fa597
Compare
|
|
||
| @test "Short form typo does not run endlessly" { | ||
| unset BATS_NO_PARALLELIZE_ACROSS_FILES | ||
| run bats -j2 "$FIXTURE_ROOT/../bats/passing.bats" |
There was a problem hiding this comment.
If you really want to test the check that was added in this PR then I think it should be:
| run bats -j2 "$FIXTURE_ROOT/../bats/passing.bats" | |
| run bats -j -2 "$FIXTURE_ROOT/../bats/passing.bats" |
Otherwise this test will switch to covering a completely different part of the code the moment you refactor the options parser.
There was a problem hiding this comment.
Additional test submitted in #693
Before commit 48b0d26, -j2 and -j -2 both triggered an infinite loop. That commit fixed the infinite loop but -j2 and -j -2 are still parsed the same. In the corresponding code review bats-core#657 Martin expressed an intent to change the parser and parse these differently in the future. In commit 0ec3619 Martin added a "no infinite loop" test for "-j2". However this test will stop testing negative arguments when the parser changes (the test will also have to be adjusted when the parser changes). Add one additional test with a "-j -3" argument that will test a negative argument forever. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Before commit 48b0d26, -j2 and -j -2 both triggered an infinite loop. That commit fixed the infinite loop but -j2 and -j -2 are still parsed the same. In the corresponding code review bats-core#657 Martin expressed an intent to change the parser and parse these differently in the future. In commit 0ec3619 Martin added a "no infinite loop" test for "-j2". However this test will stop testing negative arguments when the parser changes (the test will also have to be adjusted when the parser changes). Add one additional test with a "-j -3" argument that will test a negative argument forever. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
bats's option parser translates "-j5" to "-j -5". This caused a negative number of "slots" (threads) to be available and the test to silently hang forever. Not nice.
Add a $num_jobs sanity check to fail immediately like this:
Non-numeric arguments (e.g.: -jf) evaluate to zero which also fails the check.
Signed-off-by: Marc Herbert marc.herbert@intel.com