Bail to prevent silent failures with mocha --parallel

[echuber2]

Fixes #6940

Adding --bail seems to help prevent mocha --parallel from failing silently, but I'm not sure if this is appropriate based on how CI is expected to behave on the server.

[nwalters512]

I'm not particularly in favor of this, as it means that at most one test failure will be reported at a time. For someone trying to get tests passing, it's very valuable to get the full list up-front as opposed to only being able to see one failed test at a time.

[echuber2]

Then the line that invokes mocha could be repeated with --bail the second time to make sure the failure hits home. Unfortunately this would make CI much slower.

[echuber2]

Another idea would be to run with bail the first time, and if it fails, rerun without bail to get the full diagnostics, then force a failure result.

[nwalters512]

If we really want to fix this, I'd be inclined to try to get a minimum reproduction and report it upstream to Mocha. Both of your suggested options would indeed slow down CI substantially in the case of a failing test; it's already slow enough that I'm very hesitant to implement either one.

[echuber2]

I made the latter change for sake of reference. I'm not sure what else to suggest at this point as a workaround other than to do some form of error detection of our own to make sure the job fails if any errors occurred.

[echuber2]

We could use GNU parallel to run mocha jobs on one test file at a time instead of letting mocha handle it.

[nwalters512]

IMO it's not worth trying to work around at the moment.

[jonatanschroeder]

TBH I actually prefer the bail option, and usually when testing locally I use it. If there is more than one test failing, it is pretty common for one error to be the cause of the others (e.g., if the grade update failed, then reading the updated grade will also fail). So the second, third, etc. error are often not that useful, and when there are dozens of errors caused by a couple of problems, reporting all of them usually doesn't help.

Besides, this would typically cause these errors to be reported slightly sooner to the dev, and would save on actions running time.

[echuber2]

Actually, I think I've found the bug in mocha's parallel runner. I have a patch to it working locally. I'll try to send them a PR.

On a separate note, even when I switch to a homebrew parallel runner, there's some strange behavior with the non-parallelized mocha job runners lingering, but only for some of the jobs. That may mean there's a separate issue with the non-parallel code branch in mocha too. However it's too convoluted to sort out where the problem stems from in that situation, so I'm going to just focus on the issue with their own parallel runner.

[nwalters512]

Here's the fix that @echuber2 is proposing: mochajs/mocha#4959

[eliotwrobson]

From the discussion, it seems like we shouldn't use the fix as-written here and instead wait for the submitted PR to mocha to be merged. @nwalters512 can this PR be closed then?

[nwalters512]

Yeah, I think waiting for this to be fixed upstream is the right thing to do. FWIW, we haven't seen this issue in practice since this PR was opened. @echuber2 thanks again for looking into this and PRing a fix upstream!