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@xxxxxxxxxx> 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
