From: Phil Sutter <phil@nwl.cc>
To: Thomas Haller <thaller@redhat.com>
Cc: NetFilter <netfilter-devel@vger.kernel.org>
Subject: Re: [PATCH 1/1] tests/shell: no longer support unprettified ".json-nft" files
Date: Fri, 9 Feb 2024 16:35:04 +0100 [thread overview]
Message-ID: <ZcZGKMRKgvWsIanx@orbyte.nwl.cc> (raw)
In-Reply-To: <20240209121147.2294486-1-thaller@redhat.com>
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
prev parent reply other threads:[~2024-02-09 15:35 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-09 12:10 [PATCH 1/1] tests/shell: no longer support unprettified ".json-nft" files Thomas Haller
2024-02-09 15:35 ` Phil Sutter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZcZGKMRKgvWsIanx@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=netfilter-devel@vger.kernel.org \
--cc=thaller@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).