[PATCH nft 0/2] pretty print .json-nft files

[PATCH nft 0/2] pretty print .json-nft files

[Thomas Haller]

The plain `nft -j list ruleset` output is hard to read in diffs.
Instead, commit pretty printed JSON to git and compare that.

Existing .json-nft continue to work, and don't need to be regenerated.
But when the dump content changes, the prettified version is committed
to git.

Thomas Haller (2):
  tests/shell: use generated ruleset for `nft --check`
  tests/shell: have .json-nft dumps prettified to wrap lines

 tests/shell/helpers/json-pretty.sh  | 27 +++++++---
 tests/shell/helpers/test-wrapper.sh | 81 ++++++++++++++++++++---------
 2 files changed, 75 insertions(+), 33 deletions(-)

-- 
2.42.0

[PATCH nft 1/2] tests/shell: use generated ruleset for `nft --check`

[Thomas Haller]

The command `nft [-j] list ruleset | nft [-j] --check -f -` should never
fail. "test-wrapper.sh" already checks for that.

However, previously, we would run check against the .nft/.json-nft
files. In most cases, the generated ruleset and the files in git are
identical. However, when they are not, we (also) want to run the check
against the generated one.

This means, we can also run this check every time, regardless whether a
.nft/.json-nft file exists.

If the .nft/.json-nft file is different from the generated one, (because
a test was skipped or because there is a bug), then also check those
files. But this time, any output is ignored as failures are expected
to happen. We still run the check, to get additional coverage for
valgrind or santizers.

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

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index 4ffc48184dd7..f0170d763291 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -222,36 +222,40 @@ if [ "$rc" = 1 -o -s "$NFT_TEST_TESTTMPDIR/chkdump" ] ; then
 	show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT flush ruleset\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
 	rc_chkdump=1
 fi
-# For the dumpfiles, call `nft --check` to possibly cover new code paths.
-if [ -f "$DUMPFILE" ] ; then
-	if [ "$rc_test" -eq 77 ] ; then
-		# The test was skipped. Possibly we don't have the required
-		# features to process this file. Ignore any output and exit
-		# code, but still call the program (for valgrind or sanitizer
-		# issue we hope to find).
-		$NFT --check -f "$DUMPFILE" &>/dev/null || :
+# Check that `nft [-j] list ruleset | nft [-j] --check -f -` works.
+fail=n
+$NFT --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
+if [ "$fail" = y ] ; then
+	show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+	rc_chkdump=1
+fi
+if [ -f "$DUMPFILE" ] && ! cmp "$DUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after" &>/dev/null ; then
+	# Also check the $DUMPFILE to hit possibly new code paths. This
+	# is useful to see crashes and with ASAN/valgrind.
+	$NFT --check -f "$DUMPFILE" &>/dev/null || :
+fi
+if [ "$NFT_TEST_HAVE_json" != n ] ; then
+	if [ ! -f "$JDUMPFILE" ] ; then
+		# Optimally, `nft -j list ruleset | nft -j --check -f -` never
+		# fails.  However, there are known issues where this doesn't
+		# work, and we cannot assert hard against that. It's those
+		# tests that don't have a .json-nft file.
+		#
+		# This should be fixed, every test should have a .json-nft
+		# file, and this workaround removed.
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null || :
 	else
 		fail=n
-		$NFT --check -f "$DUMPFILE" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
 		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
 		if [ "$fail" = y ] ; then
-			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT --check -f \"$DUMPFILE\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
 			rc_chkdump=1
 		fi
-		rm -f "$NFT_TEST_TESTTMPDIR/chkdump"
 	fi
-fi
-if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
-	if [ "$rc_test" -eq 77 ] ; then
+	if [ -f "$JDUMPFILE" ] && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null ; then
 		$NFT -j --check -f "$JDUMPFILE" &>/dev/null || :
-	else
-		fail=n
-		$NFT -j --check -f "$JDUMPFILE" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
-		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
-		if [ "$fail" = y ] ; then
-			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$JDUMPFILE\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
-			rc_chkdump=1
-		fi
 	fi
 fi
 rm -f "$NFT_TEST_TESTTMPDIR/chkdump"
-- 
2.42.0

[PATCH nft 2/2] tests/shell: have .json-nft dumps prettified to wrap lines

[Thomas Haller]

Previously, the .json-nft file in git contains the output of `nft -j
list ruleset`. This is one long line and makes diffs harder to review.

Instead, have the prettified .json-nft file committed to git.

- the diff now operates on the prettified version. That means, it
  compares essentially

     - `nft -j list ruleset | json-sanitize-ruleset.sh | json-pretty.sh`
     - `cat "$TEST.json-nft" | json-pretty.sh`

  The script "json-diff-pretty.sh" is no longer used. It is kept
  however, because it might be a useful for manual comparing files.

  Note that "json-sanitize-ruleset.sh" and "json-pretty.sh" are still
  two separate scripts and called at different times. They also do
  something different. The former mangles the JSON to account for changes
  that are not stable (in the JSON data itself), while the latter only
  pretty prints it.

- when generating a new .json-nft dump file, the file will be updated to
  use the new, prettified format, unless the file is in the old format
  and needs no update. This means, with DUMPGEN=y, old style is preserved
  unless an update becomes necessary.

This requires "json-pretty.sh" having stable output, as those files are
committed to git. This is probably fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/json-pretty.sh  | 27 ++++++++++++++-----
 tests/shell/helpers/test-wrapper.sh | 41 +++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/tests/shell/helpers/json-pretty.sh b/tests/shell/helpers/json-pretty.sh
index 0d6972b81e2f..5407a8420058 100755
--- a/tests/shell/helpers/json-pretty.sh
+++ b/tests/shell/helpers/json-pretty.sh
@@ -1,17 +1,30 @@
 #!/bin/bash -e
 
-# WARNING: the output is not guaranteed to be stable.
+exec_pretty() {
+	# The output of this command must be stable (and `jq` and python
+	# fallback must generate the same output.
 
-if command -v jq &>/dev/null ; then
-	# If we have, use `jq`
-	exec jq
-fi
+	if command -v jq &>/dev/null ; then
+		# If we have, use `jq`
+		exec jq
+	fi
 
-# Fallback to python.
-exec python -c '
+	# Fallback to python.
+	exec python -c '
 import json
 import sys
 
 parsed = json.load(sys.stdin)
 print(json.dumps(parsed, indent=2))
 '
+}
+
+[ "$#" -le 1 ] || { echo "At most one argument supported" ; exit 1 ; }
+
+if [ "$#" -eq 1 ] ; then
+	# One argument passed. This must be a JSON file.
+	[ -f "$1" ] || { echo "File \"$1\" does not exist" ; exit 1 ; }
+	exec_pretty < "$1"
+fi
+
+exec_pretty
diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index f0170d763291..529dc1aada7d 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -25,6 +25,10 @@ show_file() {
 	printf "<<<<\n"
 }
 
+json_pretty() {
+	"$NFT_TEST_BASEDIR/helpers/json-pretty.sh" "$@" 2>&1 || :
+}
+
 TEST="$1"
 TESTBASE="$(basename "$TEST")"
 TESTDIR="$(dirname "$TEST")"
@@ -140,6 +144,7 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 	fi
 	# JSON output needs normalization/sanitization, otherwise it's not stable.
 	"$NFT_TEST_BASEDIR/helpers/json-sanitize-ruleset.sh" "$NFT_TEST_TESTTMPDIR/ruleset-after.json"
+	json_pretty "$NFT_TEST_TESTTMPDIR/ruleset-after.json" > "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty"
 fi
 
 read tainted_after < /proc/sys/kernel/tainted
@@ -186,7 +191,12 @@ 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
-		cat "$NFT_TEST_TESTTMPDIR/ruleset-after.json" > "$JDUMPFILE"
+		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
 	fi
 fi
 
@@ -201,12 +211,16 @@ if [ "$rc_test" -ne 77 -a "$dump_written" != y ] ; then
 		fi
 	fi
 	if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
-		if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
-			"$NFT_TEST_BASEDIR/helpers/json-diff-pretty.sh" \
-				"$JDUMPFILE" \
-				"$NFT_TEST_TESTTMPDIR/ruleset-after.json" \
-				2>&1 > "$NFT_TEST_TESTTMPDIR/ruleset-diff.json.pretty"
-			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+		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 "$NFT_TEST_TESTTMPDIR/json-nft-pretty" "$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"
 			rc_dump=1
 		else
 			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff.json"
@@ -245,6 +259,7 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 		# This should be fixed, every test should have a .json-nft
 		# file, and this workaround removed.
 		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null || :
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &>/dev/null || :
 	else
 		fail=n
 		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
@@ -253,8 +268,18 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
 			rc_chkdump=1
 		fi
+		fail=n
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
+		if [ "$fail" = y ] ; then
+			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+			rc_chkdump=1
+		fi
 	fi
-	if [ -f "$JDUMPFILE" ] && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null ; then
+	if [ -f "$JDUMPFILE" ] \
+	     && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null \
+	     && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &>/dev/null ; \
+	then
 		$NFT -j --check -f "$JDUMPFILE" &>/dev/null || :
 	fi
 fi
-- 
2.42.0

[PATCH nft 2/2 v2] tests/shell: have .json-nft dumps prettified to wrap lines

[Thomas Haller]

Previously, the .json-nft file in git contains the output of `nft -j
list ruleset`. This is one long line and makes diffs harder to review.

Instead, have the prettified .json-nft file committed to git.

- the diff now operates on the prettified version. That means, it
  compares essentially

     - `nft -j list ruleset | json-sanitize-ruleset.sh | json-pretty.sh`
     - `cat "$TEST.json-nft" | json-pretty.sh`

  The script "json-diff-pretty.sh" is no longer used. It is kept
  however, because it might be a useful for manual comparing files.

  Note that "json-sanitize-ruleset.sh" and "json-pretty.sh" are still
  two separate scripts and called at different times. They also do
  something different. The former mangles the JSON to account for changes
  that are not stable (in the JSON data itself), while the latter only
  pretty prints it.

- when generating a new .json-nft dump file, the file will be updated to
  use the new, prettified format, unless the file is in the old format
  and needs no update. This means, with DUMPGEN=y, old style is preserved
  unless an update becomes necessary.

This requires "json-pretty.sh" having stable output, as those files are
committed to git. This is probably fine.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---

Changes compared to v1:
- fix bug to use "$JDUMPFILE2" in diff command.

Still open question:
- whether to just re-generate/prettify all .json-nft files once. The
  current patch allows the files on disk to be in either form, and a
  DUMPGEN=y will only update the file, when necessary. Alternative
  would be to merge this patch, followed by regenerating all .json-nft
  files. In that case, this patch could be slightly simplified to not
  supporting the "mixed-mode".

 tests/shell/helpers/json-pretty.sh  | 27 ++++++++++++++-----
 tests/shell/helpers/test-wrapper.sh | 41 +++++++++++++++++++++++------
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/tests/shell/helpers/json-pretty.sh b/tests/shell/helpers/json-pretty.sh
index 0d6972b81e2f..5407a8420058 100755
--- a/tests/shell/helpers/json-pretty.sh
+++ b/tests/shell/helpers/json-pretty.sh
@@ -1,17 +1,30 @@
 #!/bin/bash -e
 
-# WARNING: the output is not guaranteed to be stable.
+exec_pretty() {
+	# The output of this command must be stable (and `jq` and python
+	# fallback must generate the same output.
 
-if command -v jq &>/dev/null ; then
-	# If we have, use `jq`
-	exec jq
-fi
+	if command -v jq &>/dev/null ; then
+		# If we have, use `jq`
+		exec jq
+	fi
 
-# Fallback to python.
-exec python -c '
+	# Fallback to python.
+	exec python -c '
 import json
 import sys
 
 parsed = json.load(sys.stdin)
 print(json.dumps(parsed, indent=2))
 '
+}
+
+[ "$#" -le 1 ] || { echo "At most one argument supported" ; exit 1 ; }
+
+if [ "$#" -eq 1 ] ; then
+	# One argument passed. This must be a JSON file.
+	[ -f "$1" ] || { echo "File \"$1\" does not exist" ; exit 1 ; }
+	exec_pretty < "$1"
+fi
+
+exec_pretty
diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index f0170d763291..f1f33991b454 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -25,6 +25,10 @@ show_file() {
 	printf "<<<<\n"
 }
 
+json_pretty() {
+	"$NFT_TEST_BASEDIR/helpers/json-pretty.sh" "$@" 2>&1 || :
+}
+
 TEST="$1"
 TESTBASE="$(basename "$TEST")"
 TESTDIR="$(dirname "$TEST")"
@@ -140,6 +144,7 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 	fi
 	# JSON output needs normalization/sanitization, otherwise it's not stable.
 	"$NFT_TEST_BASEDIR/helpers/json-sanitize-ruleset.sh" "$NFT_TEST_TESTTMPDIR/ruleset-after.json"
+	json_pretty "$NFT_TEST_TESTTMPDIR/ruleset-after.json" > "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty"
 fi
 
 read tainted_after < /proc/sys/kernel/tainted
@@ -186,7 +191,12 @@ 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
-		cat "$NFT_TEST_TESTTMPDIR/ruleset-after.json" > "$JDUMPFILE"
+		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
 	fi
 fi
 
@@ -201,12 +211,16 @@ if [ "$rc_test" -ne 77 -a "$dump_written" != y ] ; then
 		fi
 	fi
 	if [ "$NFT_TEST_HAVE_json" != n -a -f "$JDUMPFILE" ] ; then
-		if ! $DIFF -u "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" ; then
-			"$NFT_TEST_BASEDIR/helpers/json-diff-pretty.sh" \
-				"$JDUMPFILE" \
-				"$NFT_TEST_TESTTMPDIR/ruleset-after.json" \
-				2>&1 > "$NFT_TEST_TESTTMPDIR/ruleset-diff.json.pretty"
-			show_file "$NFT_TEST_TESTTMPDIR/ruleset-diff.json" "Failed \`$DIFF -u \"$JDUMPFILE\" \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\`" >> "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+		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"
 			rc_dump=1
 		else
 			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff.json"
@@ -245,6 +259,7 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 		# This should be fixed, every test should have a .json-nft
 		# file, and this workaround removed.
 		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null || :
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &>/dev/null || :
 	else
 		fail=n
 		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
@@ -253,8 +268,18 @@ if [ "$NFT_TEST_HAVE_json" != n ] ; then
 			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after.json\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
 			rc_chkdump=1
 		fi
+		fail=n
+		$NFT -j --check -f "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &> "$NFT_TEST_TESTTMPDIR/chkdump" || fail=y
+		test -s "$NFT_TEST_TESTTMPDIR/chkdump" && fail=y
+		if [ "$fail" = y ] ; then
+			show_file "$NFT_TEST_TESTTMPDIR/chkdump" "Command \`$NFT -j --check -f \"$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty\"\` failed" >> "$NFT_TEST_TESTTMPDIR/rc-failed-chkdump"
+			rc_chkdump=1
+		fi
 	fi
-	if [ -f "$JDUMPFILE" ] && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null ; then
+	if [ -f "$JDUMPFILE" ] \
+	     && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json" &>/dev/null \
+	     && ! cmp "$JDUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after.json-pretty" &>/dev/null ; \
+	then
 		$NFT -j --check -f "$JDUMPFILE" &>/dev/null || :
 	fi
 fi
-- 
2.43.0

Re: [PATCH nft 2/2 v2] tests/shell: have .json-nft dumps prettified to wrap lines

[Phil Sutter]

On Thu, Dec 07, 2023 at 10:08:50AM +0100, Thomas Haller wrote:
> Previously, the .json-nft file in git contains the output of `nft -j
> list ruleset`. This is one long line and makes diffs harder to review.
> 
> Instead, have the prettified .json-nft file committed to git.
> 
> - the diff now operates on the prettified version. That means, it
>   compares essentially
> 
>      - `nft -j list ruleset | json-sanitize-ruleset.sh | json-pretty.sh`
>      - `cat "$TEST.json-nft" | json-pretty.sh`
> 
>   The script "json-diff-pretty.sh" is no longer used. It is kept
>   however, because it might be a useful for manual comparing files.
> 
>   Note that "json-sanitize-ruleset.sh" and "json-pretty.sh" are still
>   two separate scripts and called at different times. They also do
>   something different. The former mangles the JSON to account for changes
>   that are not stable (in the JSON data itself), while the latter only
>   pretty prints it.
> 
> - when generating a new .json-nft dump file, the file will be updated to
>   use the new, prettified format, unless the file is in the old format
>   and needs no update. This means, with DUMPGEN=y, old style is preserved
>   unless an update becomes necessary.
> 
> This requires "json-pretty.sh" having stable output, as those files are
> committed to git. This is probably fine.
> 
> Signed-off-by: Thomas Haller <thaller@redhat.com>

Patch applied and pushed along with a bulk dump file conversion to
pretty format.

Thanks, Phil