[5/9] tests/xfs/group: add group for tests which require a logdev
diff mbox

Message ID 20171213004519.29340-6-mcgrof@kernel.org
State New
Headers show

Commit Message

Luis Chamberlain Dec. 13, 2017, 12:45 a.m. UTC
This should make it easy to run these separately or exclude them.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 tests/xfs/group | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dave Chinner Dec. 13, 2017, 9:50 p.m. UTC | #1
On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote:
> This should make it easy to run these separately or exclude them.

These should notrun automatically if you don't have an external log
device configured. Every test should either work with an external
logdev or explicitly notrun them, so I'm not sure what you're trying
to acheive here....

> 
> Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> ---
>  tests/xfs/group | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/xfs/group b/tests/xfs/group
> index d23006041ea2..cce98847de53 100644
> --- a/tests/xfs/group
> +++ b/tests/xfs/group
> @@ -42,7 +42,7 @@
>  042 fsr ioctl auto
>  043 dump ioctl tape
>  044 other auto
> -045 other auto quick
> +045 other auto quick logdev

This change also looks wrong because:

xfs/044  [not run] This test requires a valid $SCRATCH_LOGDEV
xfs/045 1s ... 1s

xfs/044 is the external logdev test, and xfs/045 is a
duplicate uuid mount test that has nothign to do with external
log devices.

And, FWIW, we already have a "log" group to indicate tests that
exercise the log, and that mostly includes all the tests that use
external logs. It would be better to tag all the tests that exercise
the log with "log" rather than create some new group that doesn't
really provide any added benefit....

Cheers,

Dave.
Luis Chamberlain Dec. 13, 2017, 11 p.m. UTC | #2
On Thu, Dec 14, 2017 at 08:50:13AM +1100, Dave Chinner wrote:
> On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote:
> > This should make it easy to run these separately or exclude them.
> 
> These should notrun automatically if you don't have an external log
> device configured. Every test should either work with an external
> logdev or explicitly notrun them, so I'm not sure what you're trying
> to acheive here....

The way I'm splitting up tests is one first run with a basic xfs section
on a configuration file, with no external log, which pretty much runs all
tests but excludes all which require external or funky configurations.

A secondary pass then goes through these extra groups and then runs tests
only for the previously excluded groups but with their own respective
section. So for instance in this case I have:

[xfs]
....
[logdev_xfs]
...

Automatic detection if the requirements are met is fine, but this doesn't 
let me easily use say:

	./check -s logdev_xfs -g logdev

> > 
> > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
> > ---
> >  tests/xfs/group | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/xfs/group b/tests/xfs/group
> > index d23006041ea2..cce98847de53 100644
> > --- a/tests/xfs/group
> > +++ b/tests/xfs/group
> > @@ -42,7 +42,7 @@
> >  042 fsr ioctl auto
> >  043 dump ioctl tape
> >  044 other auto
> > -045 other auto quick
> > +045 other auto quick logdev
> 
> This change also looks wrong because:
> 
> xfs/044  [not run] This test requires a valid $SCRATCH_LOGDEV
> xfs/045 1s ... 1s
> 
> xfs/044 is the external logdev test, and xfs/045 is a
> duplicate uuid mount test that has nothign to do with external
> log devices.

I see...

> And, FWIW, we already have a "log" group to indicate tests that
> exercise the log, and that mostly includes all the tests that use
> external logs. It would be better to tag all the tests that exercise
> the log with "log" rather than create some new group that doesn't
> really provide any added benefit....

So for my case would one better goal be to just run check without the external
one and one with the external log?

  ./check -s xfs -g log
  ./check -s logdev_xfs -g log

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Chinner Dec. 13, 2017, 11:39 p.m. UTC | #3
On Thu, Dec 14, 2017 at 12:00:52AM +0100, Luis R. Rodriguez wrote:
> On Thu, Dec 14, 2017 at 08:50:13AM +1100, Dave Chinner wrote:
> > On Tue, Dec 12, 2017 at 04:45:15PM -0800, Luis R. Rodriguez wrote:
> > > This should make it easy to run these separately or exclude them.
> > 
> > These should notrun automatically if you don't have an external log
> > device configured. Every test should either work with an external
> > logdev or explicitly notrun them, so I'm not sure what you're trying
> > to acheive here....
> 
> The way I'm splitting up tests is one first run with a basic xfs section
> on a configuration file, with no external log, which pretty much runs all
> tests but excludes all which require external or funky configurations.
> 
> A secondary pass then goes through these extra groups and then runs tests
> only for the previously excluded groups but with their own respective
> section. So for instance in this case I have:
> 
> [xfs]
> ....
> [logdev_xfs]
> ...

Which seems to me like a misguided attempt to optimise test
runtimes. i.e. this doesn't provide test coverage of external log
behaviour in all the cases that need to be tested.

Data integrity code paths are affected by having an external log. IO
ordering changes with external logs, which can expose update/crash
recovery problems. external logs can expose data IO race conditions
that are masked by interleaved log IO. etc, etc, etc.

You can't just run an internal log test then add couple of extra
external log tests and say "external logs work fine".

> Automatic detection if the requirements are met is fine, but this doesn't 
> let me easily use say:
> 
> 	./check -s logdev_xfs -g logdev

You can do that if we ignore the fact that a large number of tests
need to be run on both internal and external log devices to cover
the differences in behaviour between them.

> > And, FWIW, we already have a "log" group to indicate tests that
> > exercise the log, and that mostly includes all the tests that use
> > external logs. It would be better to tag all the tests that exercise
> > the log with "log" rather than create some new group that doesn't
> > really provide any added benefit....
> 
> So for my case would one better goal be to just run check without the external
> one and one with the external log?

See above. Your test coverage assumptions are wrong, so what you are
trying to do really doesn't tell you whether external logs work
correctly or not. It's worse that not testing external logs at all,
because it gives the false impression that they have been
exhaustively tested and work just fine when that really isn't the
case.

Cheers,

Dave.
Luis Chamberlain Dec. 14, 2017, 5:48 p.m. UTC | #4
On Thu, Dec 14, 2017 at 10:39:14AM +1100, Dave Chinner wrote:
> You can't just run an internal log test then add couple of extra
> external log tests and say "external logs work fine".
> 
> > Automatic detection if the requirements are met is fine, but this doesn't 
> > let me easily use say:
> > 
> > 	./check -s logdev_xfs -g logdev
> 
> You can do that if we ignore the fact that a large number of tests
> need to be run on both internal and external log devices to cover
> the differences in behaviour between them.
> 
> > > And, FWIW, we already have a "log" group to indicate tests that
> > > exercise the log, and that mostly includes all the tests that use
> > > external logs. It would be better to tag all the tests that exercise
> > > the log with "log" rather than create some new group that doesn't
> > > really provide any added benefit....
> > 
> > So for my case would one better goal be to just run check without the external
> > one and one with the external log?
> 
> See above. Your test coverage assumptions are wrong, so what you are
> trying to do really doesn't tell you whether external logs work
> correctly or not. It's worse that not testing external logs at all,
> because it gives the false impression that they have been
> exhaustively tested and work just fine when that really isn't the
> case.

Makes sense, thanks.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/tests/xfs/group b/tests/xfs/group
index d23006041ea2..cce98847de53 100644
--- a/tests/xfs/group
+++ b/tests/xfs/group
@@ -42,7 +42,7 @@ 
 042 fsr ioctl auto
 043 dump ioctl tape
 044 other auto
-045 other auto quick
+045 other auto quick logdev
 046 dump ioctl auto quick
 047 dump ioctl auto
 048 other auto quick
@@ -272,7 +272,7 @@ 
 272 auto quick rmap fsmap
 273 auto rmap fsmap
 274 auto quick rmap fsmap
-275 auto quick rmap fsmap
+275 auto quick rmap fsmap logdev
 276 auto quick rmap fsmap
 277 auto quick rmap fsmap
 278 repair auto