Re: [PATCH] check: fail tests if check/dmesg are not clean

Eryu Guan
2018-05-12T12:51:13Z

On Mon, May 07, 2018 at 08:46:34AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> Currently a test passes even if it leaves a corrupt filesystem
> behind, or a splat in the system logs that should not be there.

It seems that test does fail when post-test fsck fails or dmesg check
reports failure, but just after the test runtime being recorded &
reported, which makes the test looks like a PASS. But the test summary
does report it as a failure, e.g. (I added "echo BUG: > /dev/kmsg" to
generic/444 manually)

generic/443 0s ... 0s
generic/444 0s ... 0s
_check_dmesg: something found in dmesg (see /root/workspace/xfstests/results//xfs_2k_crc/generic/444.dmesg)
generic/445 0s ... 0s
Ran: generic/443 generic/444 generic/445
Failures: generic/444
Failed 1 of 3 tests

And the return value of check is also non-zero.

> Rework the check code to consider these as test failures so they can
> be accounted and tracked correctly. This also allows us to include
> the post-test filesystem checking in the test runtime - that is
> currently not accounted to the test, either, so the real runtime of
> each test is not accurately reflected in the time stats being
> reported.
>
> This requires a complete reworking of the main test check loop. It's
> a bunch of spaghetti at the moment because it has post test
> reporting code preventing use from using continue when a test is
> done. Move that post test reporting to the start of the next loop
> iteration and clean up the code to use continues where appropriate.
>
> Also, for cases where we haven't run the test or it's already been
> marked as failed, don't bother running the filesystem/dmesg checks
> for failure as we're already going to report the test as failed.
>
> This touches almost all of the loop, so get rid of the remaining
> 4 space indents inside the loop while moving all this code around.
>
> Signed-Off-By: Dave Chinner <dchinner@redhat.com>

But the whole rework looks good! Just need to reword the patch summary
and description?

Thanks,
Eryu