Notice if the test-runner dies

[nabijaczleweli]

Description

The Other Bit™ of #12990. Rich seems to've lost interest, I think there's still a point to landing this for now. Nevertheless, just like #12995, this is authored by him (which seems to've been lost on merge?).

Decided to cannibalise $REPORT_FILE for the duration between finishing the test and starting the reporter instead of making another temp file.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes. – none apply
• I have run the ZFS Test Suite with this change applied. – CI will hopefully
• All commit messages are properly formatted and contain Signed-off-by.

[szubersk]

Regular if statement would be much more readable.

[rincebrain]

I was just going to wait a few days and hope people didn't bikeshed the next time, but this works.

[behlendorf]

This change appears to result in all the TEST bots reporting failure. The original intent was to only report failures if after retrying any failed tests we still have unexpected failures.

[nabijaczleweli]

I rolled it back to only the first one (this is a direct translation of #12990 now), and it looks like they both globally fail all TEST buildds?

[rincebrain]

My original didn't, it seems. But the ones that use stashing $? do.

Curious, since the GH CI runners are happy with their lives, at least in some of mine. Shell being used difference?

...I never thought I'd suggest this, but given that the script hard fails if ksh isn't present to begin with, is there a reason for it to be /bin/sh and not /bin/ksh, or a /bin/sh thin wrapper to do the ksh symlinking on FBSD then /bin/ksh?

[nabijaczleweli]

I ran the tests in set -x mode, and the GHA test runners fail like the buildbot ones (previously this was just a spurious success), and forward non-zero results as expected, so I'm not quite sure what the intended result is, if not that (that being said, I know bugger-all about the tests, so it's not saying much)?

[rincebrain]

I suspect, looking at it harder, that the current behavior is "error if we get a nonzero return from the test runner", when the case I actually wanted to notice was "just" "the test runner got killed/died unexpectedly, but it didn't log any test failures before doing so, so RESULT works out to be 0."

I don't actually know offhand what the exit code we'd see there is, versus what we're getting from the runner normally - I wasn't expecting the runner to be returning nonzero on successful run to completion, TBH, but you can see right there in summary() that it returns 1 if anything was KILLED or FAILed and 3 if it RERAN (and 2 if there were 0 tests). So presumably we want exit codes that are outside those.

[nabijaczleweli]

Updated to -gt 3 (so signaled, 128+x, and anything above RERAN) and the buildds fail as expected now I think?

[rincebrain]

I already approved it, but I would agree that the failures seem correct. 👍

[nabijaczleweli]

Rebased, squished