On Fri, Dec 22, 2023 at 04:07:26PM +1100, David Disseldorp wrote:
> On Fri, 22 Dec 2023 11:56:22 +0900, Naohiro Aota wrote:
>
> > Test cases btrfs/208, 233, 276 does not use _filter_btrfs_subvol_delete()
> > to process "btrfs subvolume delete" command's output. So, the following
> > diff occurs even with a previous fix.
> >
> > btrfs/208 - output mismatch (see /host/btrfs/208.out.bad)
> > --- tests/btrfs/208.out 2023-12-22 02:09:18.000000000 +0000
> > +++ /host/btrfs/208.out.bad 2023-12-22 02:21:40.697036486 +0000
> > @@ -6,12 +6,12 @@
> > subvol1
> > subvol2
> > subvol3
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
> > +Delete subvolume 256 (no-commit): 'SCRATCH_MNT/subvol1'
> > After deleting one subvolume:
> > subvol2
> > ...
> >
> > Let them use the filter and fix the output accordingly.
> >
> > Signed-off-by: Naohiro Aota <naohiro.aota@xxxxxxx>
>
> Looks fine.
> Reviewed-by: David Disseldorp <ddiss@xxxxxxx>
> One minor nit...
>
> > ---
> > tests/btrfs/208 | 2 +-
> > tests/btrfs/208.out | 6 +++---
> > tests/btrfs/233 | 3 ++-
> > tests/btrfs/233.out | 4 ++--
> > tests/btrfs/276 | 3 ++-
> > tests/btrfs/276.out | 2 +-
> > 6 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/tests/btrfs/208 b/tests/btrfs/208
> > index 909f9fa40803..d58803e2f801 100755
> > --- a/tests/btrfs/208
> > +++ b/tests/btrfs/208
> > @@ -28,7 +28,7 @@ _delete_and_list()
> > local msg="$2"
> >
> > SUBVOLID=$(_btrfs_get_subvolid $SCRATCH_MNT "$subvol_name")
> > - $BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_scratch
> > + $BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_btrfs_subvol_delete
> >
> > echo "$msg"
> > $BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | $AWK_PROG '{ print $NF }'
> > diff --git a/tests/btrfs/208.out b/tests/btrfs/208.out
> > index 9b660699a4b2..dc5761ba1c87 100644
> > --- a/tests/btrfs/208.out
> > +++ b/tests/btrfs/208.out
> > @@ -6,12 +6,12 @@ Current subvolume ids:
> > subvol1
> > subvol2
> > subvol3
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
> > +Delete subvolume 'SCRATCH_MNT/subvol1'
> > After deleting one subvolume:
> > subvol2
> > subvol3
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol3'
> > +Delete subvolume 'SCRATCH_MNT/subvol3'
> > Last remaining subvolume:
> > subvol2
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol2'
> > +Delete subvolume 'SCRATCH_MNT/subvol2'
> > All subvolumes removed.
> > diff --git a/tests/btrfs/233 b/tests/btrfs/233
> > index 2b94a9c6befe..f2c1eba090be 100755
> > --- a/tests/btrfs/233
> > +++ b/tests/btrfs/233
> > @@ -21,6 +21,7 @@ _cleanup()
> >
> > # Import common functions.
> > . ./common/filter
> > +. ./common/filter.btrfs
>
> common/filter.btrfs sources common/filter, so you can replace these.
Oh, I didn't notice that. But, there are many test cases importing
both. Maybe, it's good to express direct dependency (e.g, for
_filter_scratch) explicitly? Or, they should be fixed as well?
$ grep -E 'filter$' $(grep -lr filter.btrfs tests|sort)
tests/btrfs/001:. ./common/filter
tests/btrfs/035:. ./common/filter
tests/btrfs/048:. ./common/filter
tests/btrfs/059:. ./common/filter
tests/btrfs/089:. ./common/filter
tests/btrfs/096:. ./common/filter
tests/btrfs/100:. ./common/filter
tests/btrfs/101:. ./common/filter
tests/btrfs/112:. ./common/filter
tests/btrfs/113:. ./common/filter
tests/btrfs/162:. ./common/filter
tests/btrfs/163:. ./common/filter
tests/btrfs/171:. ./common/filter
tests/btrfs/197:. ./common/filter
tests/btrfs/198:. ./common/filter
tests/btrfs/208:. ./common/filter
tests/btrfs/218:. ./common/filter
tests/btrfs/233:. ./common/filter
tests/btrfs/254:. ./common/filter
tests/btrfs/276:. ./common/filter
