[PATCH 1/1] tests/shell: no longer support unprettified ".json-nft" files

[PATCH 1/1] tests/shell: no longer support unprettified ".json-nft" files

[Thomas Haller]

By now, all ".json-nft" files are prettified and will be generated in
that form.

Drop the fallback code that accepts them in the previous form.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/test-wrapper.sh | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index f1f33991b454..c016e0ce1d39 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -191,12 +191,7 @@ if [ "$rc_test" -eq 0 -a '(' "$DUMPGEN" = all -o "$DUMPGEN" = y ')' ] ; then
 		cat "$NFT_TEST_TESTTMPDIR/ruleset-after" > "$DUMPFILE"
 	fi
 	if [ "$NFT_TEST_HAVE_json" != n -a "$gen_jdumpfile" = y ] ; then
-		if cmp "$NFT_TEST_TESTTMPDIR/ruleset-after.json" "$JDUMPFILE" &>/dev/null ; then
-			# The .json-nft file is still the non-pretty variant. Keep it.
-			:
-		else
-			cat "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" > "$JDUMPFILE"
-		fi
+		cat "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" > "$JDUMPFILE"
 	fi
 fi
 
@@ -211,16 +206,8 @@ if [ "$rc_test" -ne 77 -a "$dump_written" != y ] ; then
 		fi
 	fi
 	if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
-		JDUMPFILE2="$NFT_TEST_TESTTMPDIR/json-nft-pretty"
-		json_pretty "$JDUMPFILE" > "$JDUMPFILE2"
-		if cmp "$JDUMPFILE" "$JDUMPFILE2" &>/dev/null ; then
-			# The .json-nft file is already prettified. We can use
-			# it directly.
-			rm -rf "$JDUMPFILE2"
-			JDUMPFILE2="$JDUMPFILE"
-		fi
-		if ! $DIFF -u "$JDUMPFILE2" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
-			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE2\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+		if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
+			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
 			rc_dump=1
 		else
 			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff.json"
-- 
2.43.0

Re: [PATCH 1/1] tests/shell: no longer support unprettified ".json-nft" files

[Phil Sutter]

On Fri, Feb 09, 2024 at 01:10:39PM +0100, Thomas Haller wrote:
> By now, all ".json-nft" files are prettified and will be generated in
> that form.
> 
> Drop the fallback code that accepts them in the previous form.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>

Patch applied, thanks. Some comments though:

> @@ -211,16 +206,8 @@ if [ "$rc_test" -ne 77 -a "$dump_written" != y ] ; then
>  		fi
>  	fi
>  	if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
> -		JDUMPFILE2="$NFT_TEST_TESTTMPDIR/json-nft-pretty"
> -		json_pretty "$JDUMPFILE" > "$JDUMPFILE2"
> -		if cmp "$JDUMPFILE" "$JDUMPFILE2" &>/dev/null ; then
> -			# The .json-nft file is already prettified. We can use
> -			# it directly.
> -			rm -rf "$JDUMPFILE2"
> -			JDUMPFILE2="$JDUMPFILE"
> -		fi
> -		if ! $DIFF -u "$JDUMPFILE2" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
> -			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE2\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
> +		if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
> +			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"

When playing with with changes to avoid the ~200 column lines this patch
adds, I checked what show_file actually print in addition to the
contents of ruleset-diff.json. It is (from one random example on disk):

| Failed `/usr/bin/diff -u "/tmp/nft-test.20240208-164915.277.rXP2ui/test-testcases-sets-0049set_define_0.79/json-nft-pretty" "/tmp/nft-test.20240208-164915.277.rXP2ui/test-testcases-sets-0049set_define_0.79/ruleset-after.json-pretty"`

The only non-trivial data this contains is the temp dir name
(/tmp/nft-test.20240208-164915.277.rXP2ui) and the test name
(test-testcases-sets-0049set_define_0.79). Said line yet stems from
/tmp/nft-test.20240208-164915.277.rXP2ui/test-testcases-sets-0049set_define_0.79/rc-failed-dump
so all this info is present in the file's path already.

Moreover, the diff's header in that file states the full paths to the
diffed files again. This is too much redundant data or noise IMO. So
much, I'd axe the whole show_file() stuff.

Cheers, Phil