On Mon, Apr 08, 2024 at 12:46:18PM +0800, Anand Jain wrote: > On 4/6/24 13:18, Qu Wenruo wrote: > > [BUG] > > All the touched test cases would fail after btrfs-progs commit > > 5f87b467a9e7 ("btrfs-progs: subvolume: output the prompt line only when > > the ioctl succeeded") due to golden output mismatch. > > > > [CAUSE] > > Although the patch I sent to the mail list doesn't change the output at > > all but only a timing change, David uses this patch to unify the output > > of "btrfs subvolume create" and "btrfs subvolume snapshot". > > > > Unfortunately this changes the output and causes mismatch with > > golden output. > > > > [FIX] > > Just redirect stdout of "btrfs subvolume snapshot" to $seqres.full. > > Any error from "btrfs subvolume" subgroup would lead to error messages > > into stderr, and cause golden output mismatch. > > > > This can be comprehensively greped by > > 'grep -IR "Create a" tests/btrfs/*.out' command. > > > > In fact, we have around 274 "btrfs subvolume snapshot|create" calls in the > > existing test cases, meanwhile only around 61 calls are populating > > golden output (22 for subvolume creation, and 39 for snapshot creation). > > > > Thus majority of the snapshot/subvolume creation is not populating > > golden output already. > > > > While golden output is better verification method in terms of > accuracy, but, it falls short in verifying command exit codes. > I personally think the run_btrfs_progs approach is better for > 'btrfs subvolume snapshot'. It allows us to verify the command > status without relying on the stdout. > But, past discussions favored the golden output verification > method instead of run_btrfs_progs. I thought the whole point here was to depart from the golden output, at least in selected cases, and only in btrfs/ subdirectory so it does not accidentally break other filesystems' testing. 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.