Re: [PATCH 2/2] fstests: use _scratch_mount_nocheck where appropriate in tests

[Eryu Guan <eguan@redhat.com>]

On Tue, Feb 13, 2018 at 03:31:40PM -0800, Darrick J. Wong wrote:
> On Tue, Feb 13, 2018 at 05:12:03PM +0800, Eryu Guan wrote:
> > Also remove redundant status checks of _scratch_mount.
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> 
> <skipping to the generic and xfs tests>
> 
> > diff --git a/tests/generic/054 b/tests/generic/054
> > index 12f471a19090..84b8271a11bd 100755
> > --- a/tests/generic/054
> > +++ b/tests/generic/054
> > @@ -83,7 +83,7 @@ for s in sync nosync ; do
> >  
> >  	# mount the FS
> >  	_echofull "mount"
> > -	if ! _scratch_mount >>$seqres.full 2>&1; then
> > +	if ! _scratch_mount_nocheck >>$seqres.full 2>&1; then
> >  	    _echofull "mount failed: $MOUNT_OPTIONS"
> >  	    continue
> >  	fi
> > @@ -118,7 +118,7 @@ for s in sync nosync ; do
> >  	_print_logstate
> >  
> >  	_echofull "mount with replay"
> > -	_scratch_mount >>$seqres.full 2>&1 \
> > +	_scratch_mount_nocheck >>$seqres.full 2>&1 \
> >  	    || _fail "mount failed: $MOUNT_OPTIONS"
> 
> nocheck || fail ??
> 
> I wonder if the fail message should always spit out the mount options
> that were tried?

Seems there's no need to do so in this case, as we know what
$MOUNT_OPTIONS we're using. We could dump the specific mount options if
the test uses non-default options and really cares about it, and this
should be done case-by-case.

> 
> > diff --git a/tests/generic/068 b/tests/generic/068
> > index bd12278cdd25..91b327866c54 100755
> > --- a/tests/generic/068
> > +++ b/tests/generic/068
> > @@ -62,7 +62,7 @@ echo "*** MKFS ***"                         >>$seqres.full
> >  echo ""                                     >>$seqres.full
> >  _scratch_mkfs                               >>$seqres.full 2>&1 \
> >      || _fail "mkfs failed"
> > -_scratch_mount                              >>$seqres.full 2>&1 \
> > +_scratch_mount_nocheck                      >>$seqres.full 2>&1 \
> >      || _fail "mount failed"
> 
> Can this remain _scratch_mount, since we _fail anyway?
> 
> (There are more of these scattered throughout.)

Sure. I thought it'd be better to keep dumping mount error message to
$seqres.full and keep the pretty format :) But seems that's not
necessary, the error message would break golden image anyway.

I'll revisit the tests with similar patterns.

Thanks,
Eryu