All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eryu Guan <guaneryu@gmail.com>
To: Dave Chinner <david@fromorbit.com>
Cc: fstests@vger.kernel.org
Subject: Re: [PATCH] check: fail tests if check/dmesg are not clean
Date: Sat, 12 May 2018 20:51:08 +0800	[thread overview]
Message-ID: <20180512125108.GS8373@desktop> (raw)
In-Reply-To: <20180506224634.1425-1-david@fromorbit.com>

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

  reply	other threads:[~2018-05-12 12:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06 22:46 [PATCH] check: fail tests if check/dmesg are not clean Dave Chinner
2018-05-12 12:51 ` Eryu Guan [this message]
2018-05-22  8:00   ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2018-02-23  1:16 Dave Chinner
2018-02-23  4:34 ` Eryu Guan
2018-02-26 10:10 ` Eryu Guan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180512125108.GS8373@desktop \
    --to=guaneryu@gmail.com \
    --cc=david@fromorbit.com \
    --cc=fstests@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.