Re: [PATCH 4/8] fstests: define a common _dump_cleanup function

[Dave Chinner <david@fromorbit.com>]

On Tue, May 24, 2022 at 12:04:36PM +0300, Amir Goldstein wrote:
> On Tue, May 24, 2022 at 11:32 AM Dave Chinner <david@fromorbit.com> wrote:
> > diff --git a/tests/xfs/026 b/tests/xfs/026
> > index 18529003..0daa7c88 100755
> > --- a/tests/xfs/026
> > +++ b/tests/xfs/026
> > @@ -9,17 +9,8 @@
> >  . ./common/preamble
> >  _begin_fstest dump ioctl auto quick
> >
> > -status=0       # success is the default!
> > -
> 
> All those tests that you change from success is the default
> to failure is the default, that makes sense, but
> 1. Should be documented in the commit message?

I cleaned up a bunch of random stuff like this as I went along. If I
try to document every little change that gets done, I'll be spending
more time on describing the changes than actually doing them. I
don't have the time to rigourously document this stuff - I've got
the time to modify the code, write an overview of the changes and
that's about it.

What does the fstests community want: better test infrastructure or
perfect commits?

> 2. Why were those tests written this way? Do you know?

That's just the pattern that was used by the team that wrote these
dump tests 20-odd years ago. At the time nobody cared that much
about little inconsistencies like this as long as the test failed
when something went wrong. i.e. test works, merge it, move on to the
next thing. 

Now that we have ~1700 tests to maintain, little inconsistencies are
a big deal. That's why I cleaned up the simple ones as I saw them...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com