From: Dave Chinner <david@fromorbit.com>
To: "Lukáš Czerner" <lczerner@redhat.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] 260: Add another corner case where length is zero
Date: Wed, 10 Oct 2012 08:39:13 +1100 [thread overview]
Message-ID: <20121009213913.GQ23644@dastard> (raw)
In-Reply-To: <alpine.LFD.2.00.1210092320480.2326@(none)>
On Tue, Oct 09, 2012 at 11:23:13PM +0200, Lukáš Czerner wrote:
> On Wed, 10 Oct 2012, Dave Chinner wrote:
>
> > Date: Wed, 10 Oct 2012 06:40:19 +1100
> > From: Dave Chinner <david@fromorbit.com>
> > To: Lukas Czerner <lczerner@redhat.com>
> > Cc: xfs@oss.sgi.com
> > Subject: Re: [PATCH] 260: Add another corner case where length is zero
> >
> > On Tue, Oct 09, 2012 at 02:16:52PM +0200, Lukas Czerner wrote:
> > > This commit adds another corner case to test FITRIM argument handling.
> > > In this case we set length to zero and we expect the number of discarded
> > > bytes to be obviously zero, however we've had bug in both ext4 and xfs
> > > where the internal variable would underflow. This test case will be able
> > > to catch that in future.
> > >
> > > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> >
> > I'd create another test for this, rather than making 260 suddenly
> > fail for everyone....
>
> Hmm, I am not sure what is the point. I've created 260 exactly for
> this reason of testing FITRIM argument handling and it already
> contains number of tests like this one. I am not strongly against
> having this in separate test, however it seems rather unnecessary to
> me.
IT's a regression test - it's only supposed to start failing when
the kernel functionality is broken. That is, someone who is
tracking failures over time will suddenly see a new failure in 260
and wonder what kernel code broke, when in fact nothing was changed
in the kernel code. IOWs, changing the test invalidates all past
history of running the test, and that in turn breaks historic
regression tracking metrics...
This is why we historically have avoided changing existing tests and
instead wrote new tests, no matter how similar the functionality
between the old and new tests are.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2012-10-09 21:37 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-09 12:16 [PATCH] 260: Add another corner case where length is zero Lukas Czerner
2012-10-09 13:39 ` Carlos Maiolino
2012-10-09 19:40 ` Dave Chinner
2012-10-09 21:23 ` Lukáš Czerner
2012-10-09 21:39 ` Dave Chinner [this message]
2012-10-10 6:53 ` Lukáš Czerner
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=20121009213913.GQ23644@dastard \
--to=david@fromorbit.com \
--cc=lczerner@redhat.com \
--cc=xfs@oss.sgi.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox