netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Haller <thaller@redhat.com>
To: Phil Sutter <phil@nwl.cc>
Cc: NetFilter <netfilter-devel@vger.kernel.org>
Subject: Re: [PATCH nft 1/1] tests/shell: sanitize "handle" in JSON output
Date: Tue, 21 Nov 2023 13:58:41 +0100	[thread overview]
Message-ID: <31ff0aceab627a9838b94fc3fa58c271bc0a6023.camel@redhat.com> (raw)
In-Reply-To: <ZVylBpvC+IK4RIyH@orbyte.nwl.cc>

On Tue, 2023-11-21 at 13:39 +0100, Phil Sutter wrote:
> On Tue, Nov 21, 2023 at 01:10:11PM +0100, Thomas Haller wrote:
> > On Sat, 2023-11-18 at 03:36 +0100, Phil Sutter wrote:
> [...]
> > > Also, scoping these replacements to line 1 is funny with single
> > > line
> > > input. Worse is identifying the change in the resulting diff.
> > > Maybe
> > > write a helper in python which lets you more comfortably sanitize
> > > input,
> > > sort attributes by key and output pretty-printed?
> > 
> > You mean, to parse and re-encode the JSON? That introduces
> > additional
> > changes, which seems undesirable. That's why the regex is limited
> > to
> > the first line (even if we only expect to ever see one line there).
> > 
> > Also, normalization via 2 regex seems simpler than writing some
> > python.
> > 
> > Well, pretty-printing the output with `jq` would have the
> > advantage,
> > that future diffs might be smaller (changing individual lines, vs.
> > replace one large line). Still, I think it's better to keep the
> > amount
> > of post-processing minimal.
> 
> The testsuite relies upon Python and respective modules already,
> using
> jq introduces a new dependency. Hence why I suggested to write a
> script.
> 
> JSON object attributes are not bound to any ordering, the code may
> change it.

Don't have .nft dumps the same concern?

In JSON the order of things certainly matters. libjansson has
JSON_PRESERVE_ORDER, which is used by libnftables. Also,
JSON_PRESERVE_ORDER is deprecated since 2016 and order is always
preserved.

If the order changes, that should be visible (in form of a test
failure).

> 
> When analyzing testsuite failures, a diff of two overlong lines is
> inconvenient to the point that one may pipe both through json_pp and
> then diff again. The testsuite may do just that in case of offending
> output, but the problem of reordered attributes remains.
> 
> I'd really appreciate if testsuite changes prioritized usability. I
> rather focus on fixing bugs instead of parsing the testsuite results.

The test suite prioritizes usability. No need to suggest otherwise.

To make debugging easier, the test suite can additionally show a
prettified diff. It does not determine how the .json-nft file is stored
in git. 


Thomas


  reply	other threads:[~2023-11-21 12:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-17 17:18 [PATCH nft 1/1] tests/shell: sanitize "handle" in JSON output Thomas Haller
2023-11-18  2:36 ` Phil Sutter
2023-11-21 12:10   ` Thomas Haller
2023-11-21 12:39     ` Phil Sutter
2023-11-21 12:58       ` Thomas Haller [this message]
2023-11-21 13:40         ` Phil Sutter
2023-11-21 14:35           ` Thomas Haller
2023-11-21 15:19             ` Phil Sutter
2023-11-21 12:45     ` Pablo Neira Ayuso
     [not found]       ` <20231121132331.3401846-1-thaller@redhat.com>
2023-11-22 10:36         ` [PATCH nft v3 " Pablo Neira Ayuso
2023-11-22 10:44           ` Thomas Haller
2023-11-22 11:22             ` Pablo Neira Ayuso
2023-11-22 12:16               ` Thomas Haller
2023-11-22 12:32                 ` Pablo Neira Ayuso

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=31ff0aceab627a9838b94fc3fa58c271bc0a6023.camel@redhat.com \
    --to=thaller@redhat.com \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    /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).