Re: [PATCH] btrfs-progs: fix a check condition in misc/038

[Josef Bacik <josef@toxicpanda.com>]

On Tue, Jun 04, 2024 at 03:30:00PM +0930, Qu Wenruo wrote:
> The test case always fail in my VM, with the following error:
> 
>  $ sudo TEST=038\* make test-misc
>     [TEST]   misc-tests.sh
>     [TEST/misc]   038-backup-root-corruption
>  Backup 2 not overwritten
>  test failed for case 038-backup-root-corruption
> 
> After more debugging, the it turns out that there is nothing wrong
> except the final check:
> 
>  [ "$main_root_ptr" -ne "$backup_new_root_ptr" ] || _fail "Backup 2 not overwritten"
> 
> The _fail() is only triggered if the previous check returns false, which
> is completely the opposite.
> 
> In fact the "[ check ] || _fail" pattern is the worst thing in the bash
> world, super easy to cause the opposite check condition.
> 

Except we do this all of the time, we should be used to it by now.

> Fix it by use a proper "if []; then fi" block, and since we're here also
> update the error message to use the newest slot number instead.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/misc-tests/038-backup-root-corruption/test.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/misc-tests/038-backup-root-corruption/test.sh b/tests/misc-tests/038-backup-root-corruption/test.sh
> index 9be0cee36239..0f97577849cc 100755
> --- a/tests/misc-tests/038-backup-root-corruption/test.sh
> +++ b/tests/misc-tests/038-backup-root-corruption/test.sh
> @@ -61,4 +61,6 @@ main_root_ptr=$(dump_super | awk '/^root\t/{print $2}')
>  slot_num=$(( ($slot_num + 1) % 4 ))
>  backup_new_root_ptr=$(dump_super | grep -A1 "backup $slot_num" | grep backup_tree_root | awk '{print $2}')
>  
> -[ "$main_root_ptr" -ne "$backup_new_root_ptr" ] || _fail "Backup 2 not overwritten"
> +if [ "$main_root_ptr" -ne "$backup_new_root_ptr" ]; then
> +	_fail "Backup ${slot_num} not overwritten"

Don't we prefer just "$slot_num"?  I feel like I've gotten yelld at for this
before.  Just change the existing thing to be correct

[ "$main_root_ptr" -eq "$backup_new_root_ptr" ] || _fail "Backup $slot_num not overwritten"

Thanks,

Josef