Test-More / Test2-Harness

Alternative to Test::Harness
Other
23 stars 26 forks source link

Fix -j option #164

Closed dkechag closed 4 years ago

dkechag commented 4 years ago

Closes #163

Using the -j option, the first time a task finished, a $state->advance was done without a $state->next_task, so a parallel thread was essentially lost.

I added a quick test based on the existing one, although it is not nice (it makes an assumption that the tests used cant go from start to exit immediately), so maybe a nicer one should be written?

PS. The -j/jobs nomenclature (instead something like threads/parallel etc) is a bit unfortunate I realized when reading the code, but it was inherited from prove of course ;)

exodist commented 4 years ago

@dkechag I took the test you wrote, but made an alternate solution in the same area where your solution was written. You were correct about the bug, but the solution you wrote missed the fact that the while loop was intended to let the process advance as many things as it could while it had the lock. The solution I added fixes the bug, but keeps the desired loop behavior.

exodist commented 4 years ago

Please verify this fixes the problem on your end, assuming it does I can mint a new release.

dkechag commented 4 years ago

Yes, your commit seems to work fine, thanks. Completely unrelated, but to not messaging separately: you have left one mention of HARNESS-CATEGORY-LONG in the doc, which is what was proposed originally instead of HARNESS-DURATION-LONG. Do you intend for both to continue work interchangeably? If yes, maybe add a mention of the synonym under HARNESS-DURATION-LONG (that CATEGORY-LONG is the same), otherwise replace the mention of CATEGORY-LONG?