From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Anand Jain <anand.jain@oracle.com>, Qu Wenruo <wqu@suse.com>,
linux-btrfs@vger.kernel.org, fstests@vger.kernel.org
Subject: Re: [PATCH v2] fstests: btrfs: redirect stdout of "btrfs subvolume snapshot" to fix output change
Date: Thu, 11 Apr 2024 16:33:50 +0200 [thread overview]
Message-ID: <20240411143350.GP3492@suse.cz> (raw)
In-Reply-To: <ac9c80d0-bc78-459b-aa38-4f2b6ed34b65@gmx.com>
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.
next prev parent reply other threads:[~2024-04-11 14:41 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-06 5:18 [PATCH v2] fstests: btrfs: redirect stdout of "btrfs subvolume snapshot" to fix output change Qu Wenruo
2024-04-08 4:46 ` Anand Jain
2024-04-08 5:16 ` Qu Wenruo
2024-04-09 11:13 ` David Sterba
2024-04-10 4:16 ` Anand Jain
2024-04-10 5:48 ` Qu Wenruo
2024-04-10 16:26 ` David Sterba
2024-04-11 8:44 ` Anand Jain
2024-04-11 8:49 ` Qu Wenruo
2024-04-11 14:33 ` David Sterba [this message]
2024-04-09 11:25 ` David Sterba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240411143350.GP3492@suse.cz \
--to=dsterba@suse.cz \
--cc=anand.jain@oracle.com \
--cc=fstests@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=quwenruo.btrfs@gmx.com \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).