| Message ID | 162078494495.3302755.13327851823592717788.stgit@magnolia (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fstests: miscellaneous fixes | expand |
On Tue, May 11, 2021 at 07:02:24PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Fix the obviously incorrect code here that wants to fail the test if > mkfs doesn't succeed. The return value ("$?") is always the status of > the /last/ command in the pipe. Change the checker to _notrun so that > we don't leave the scratch check files around. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > tests/xfs/178 | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > > diff --git a/tests/xfs/178 b/tests/xfs/178 > index a24ef50c..bf72e640 100755 > --- a/tests/xfs/178 > +++ b/tests/xfs/178 > @@ -57,8 +57,8 @@ _supported_fs xfs > # fix filesystem, new mkfs.xfs will be fine. > > _require_scratch > -_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs \ > - || _fail "mkfs failed!" > +_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs > +test "${PIPESTATUS[0]}" -eq 0 || _notrun "mkfs failed!" I still don't understand why changing this to _notrun, shouldn't creating a default filesystem should always pass? and fail the test if mkfs failed? Thanks, Eryu > > # By executing the followint tmp file, will get on the mkfs options stored in > # variables >
On Sun, May 16, 2021 at 11:54:30PM +0800, Eryu Guan wrote: > On Tue, May 11, 2021 at 07:02:24PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Fix the obviously incorrect code here that wants to fail the test if > > mkfs doesn't succeed. The return value ("$?") is always the status of > > the /last/ command in the pipe. Change the checker to _notrun so that > > we don't leave the scratch check files around. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > tests/xfs/178 | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/tests/xfs/178 b/tests/xfs/178 > > index a24ef50c..bf72e640 100755 > > --- a/tests/xfs/178 > > +++ b/tests/xfs/178 > > @@ -57,8 +57,8 @@ _supported_fs xfs > > # fix filesystem, new mkfs.xfs will be fine. > > > > _require_scratch > > -_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs \ > > - || _fail "mkfs failed!" > > +_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs > > +test "${PIPESTATUS[0]}" -eq 0 || _notrun "mkfs failed!" > > I still don't understand why changing this to _notrun, shouldn't creating a > default filesystem should always pass? It will, unless you've injected a malicious mkfs.xfs to make sure constructions like these actually work. > and fail the test if mkfs failed? Yeah, you're right, it /should/ fail. I'll leave it alone then. --D > > Thanks, > Eryu > > > > > # By executing the followint tmp file, will get on the mkfs options stored in > > # variables > >
diff --git a/tests/xfs/178 b/tests/xfs/178 index a24ef50c..bf72e640 100755 --- a/tests/xfs/178 +++ b/tests/xfs/178 @@ -57,8 +57,8 @@ _supported_fs xfs # fix filesystem, new mkfs.xfs will be fine. _require_scratch -_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs \ - || _fail "mkfs failed!" +_scratch_mkfs_xfs | _filter_mkfs 2>$tmp.mkfs +test "${PIPESTATUS[0]}" -eq 0 || _notrun "mkfs failed!" # By executing the followint tmp file, will get on the mkfs options stored in # variables