Re: [PATCH v2] fstests: btrfs: redirect stdout of "btrfs subvolume snapshot" to fix output change

[David Sterba <dsterba@suse.cz>]

On Thu, Apr 11, 2024 at 06:19:03PM +0930, Qu Wenruo wrote:
> 
> 
> 在 2024/4/11 18:14, Anand Jain 写道:
> >
> >
> > On 4/11/24 00:26, David Sterba wrote:
> >> On Wed, Apr 10, 2024 at 03:18:49PM +0930, Qu Wenruo wrote:
> >>>>> What past discussions favored does not seem to satisfy our needs
> >>>>> and as
> >>>>> btrfs-progs are evolving we're hitting random test breakage just
> >>>>> because
> >>>>> some message has changed. The testsuite should verify what matters,
> >>>>> ie.
> >>>>> return code, state of the filesystem etc, not exact command output.
> >>>>> There's high correlation between output and correctness, yes, but this
> >>>>> is too fragile.
> >>>>
> >>>> Agreed. So, why don't we use `_run_btrfs_util_prog subvolume
> >>>> snapshot`, which makes it consistent with the rest of the test cases,
> >>>> and also remove the golden output for this command?
> >>>
> >>> For `_run_btrfs_util_prog`, the only thing I do not like is the name
> >>> itself.
> >>>
> >>> I also do not like how fstests always go $BTRFS_UTIL_PROG neither,
> >>> however I understand it's there to make sure we do not got weird bash
> >>> function name like "btrfs()" overriding the real "btrfs".
> >>>
> >>> If we can make the name shorter like `_btrfs` or something like it, I'm
> >>> totally fine with that, and would be happy to move to the new interface.
> >>>
> >>> In fact, `_run_btrfs_util_prog` is pretty helpful to generate a debug
> >>> friendly seqres.full, which is another good point.
> >>
> >> I did not realize the _run_btrfs_util_prog helper was there and actually
> >> the run_check as well. I vaguely remember this from many years ago and
> >> this somehow landed in btrfs-progs testsuite but fstests was against it.
> >> Using such helpers sounds like a plan to me (with renames etc).
> >
> > We can do the renaming part in the separate patch. Qu, are
> > you sending the revised patch?
> 
> Sure, I can prepare them pretty soon.
> 
> Just to be noticed, if we really determine to rename
> `_run_btrfs_util_prog`, it would be pretty large as there are tons of
> such calls still there.
> 
> And I really hope we can get a good naming before doing the conversion.
> Updating it again and again for a different name may not be a good use
> of time.
> 
> My candidate is `_btrfs` or `_run_btrfs`. Any good candidates?

I'd use the one with prefix.