Message ID | 20231031185449.1033380-1-thaller@redhat.com |
---|---|
Headers | show |
Series | add and check dump files for JSON in tests/shell | expand |
On Tue, 2023-10-31 at 19:53 +0100, Thomas Haller wrote: > Like we have .nft dump files to compare the expected result, add > .json-nft files that compare the JSON output. > > Thomas Haller (7): > json: fix use after free in table_flags_json() > json: drop messages "warning: stmt ops chain have no json callback" > tests/shell: check and generate JSON dump files > tests/shell: add JSON dump files > tools: simplify error handling in "check-tree.sh" by adding > msg_err()/msg_warn() > tools: check more strictly for bash shebang in "check-tree.sh" > tools: check for consistency of .json-nft dumps in "check-tree.sh" Hm. Patch 4/7 bounced (too large). Will see how to resend, after there is some feedback. The patch is also here: https://gitlab.freedesktop.org/thaller/nftables/-/commit/6545b31080036e8525be5c80c0103a1509e698e4 Thomas
On Tue, Oct 31, 2023 at 08:17:24PM +0100, Thomas Haller wrote: > On Tue, 2023-10-31 at 19:53 +0100, Thomas Haller wrote: > > Like we have .nft dump files to compare the expected result, add > > .json-nft files that compare the JSON output. > > > > Thomas Haller (7): > > json: fix use after free in table_flags_json() > > json: drop messages "warning: stmt ops chain have no json callback" > > tests/shell: check and generate JSON dump files > > tests/shell: add JSON dump files > > tools: simplify error handling in "check-tree.sh" by adding > > msg_err()/msg_warn() > > tools: check more strictly for bash shebang in "check-tree.sh" > > tools: check for consistency of .json-nft dumps in "check-tree.sh" If this is improving json support coverage without imposing any extra restriction other than adding a .nft-json file, then this is very good to have. I believe I switfly read on a commit message that this is skipped if nft is compiled without json support, correct? > Hm. Patch 4/7 bounced (too large). > > Will see how to resend, after there is some feedback. I suggest you Cc: me so I can apply this. > The patch is also here: > https://gitlab.freedesktop.org/thaller/nftables/-/commit/6545b31080036e8525be5c80c0103a1509e698e4 You said: "Note that for some JSON dumps, `nft -f --check` fails (or prints something). For those tests no *.json-nft file is added. The bugs needs to be fixed first." Do you have a list of tests that are failing? Or maybe include this list in the commit description? To keep them in the radar, we can incrementally fix them. Thanks.
On Wed, 2023-11-01 at 09:28 +0100, Pablo Neira Ayuso wrote: > On Tue, Oct 31, 2023 at 08:17:24PM +0100, Thomas Haller wrote: > > On Tue, 2023-10-31 at 19:53 +0100, Thomas Haller wrote: > > > Like we have .nft dump files to compare the expected result, add > > > .json-nft files that compare the JSON output. > > > > > > Thomas Haller (7): > > > json: fix use after free in table_flags_json() > > > json: drop messages "warning: stmt ops chain have no json > > > callback" > > > tests/shell: check and generate JSON dump files > > > tests/shell: add JSON dump files > > > tools: simplify error handling in "check-tree.sh" by adding > > > msg_err()/msg_warn() > > > tools: check more strictly for bash shebang in "check-tree.sh" > > > tools: check for consistency of .json-nft dumps in "check- > > > tree.sh" > > If this is improving json support coverage without imposing any extra > restriction other than adding a .nft-json file, then this is very > good > to have. > > I believe I switfly read on a commit message that this is skipped if > nft is compiled without json support, correct? Yes, that's how it's supposed to work (and tests correctly for me). > > > Hm. Patch 4/7 bounced (too large). > > > > Will see how to resend, after there is some feedback. > > I suggest you Cc: me so I can apply this. You could also run git fetch https://gitlab.freedesktop.org/thaller/nftables.git 6545b31080036e8525be5c80c0103a1509e698e4 ; git cherry-pick 6545b31080036e8525be5c80c0103a1509e698e4 Anyway. I will CC you on the patch as soon as I get to it! > > > The patch is also here: > > https://gitlab.freedesktop.org/thaller/nftables/-/commit/6545b31080036e8525be5c80c0103a1509e698e4 > > You said: > > "Note that for some JSON dumps, `nft -f --check` fails (or prints > something). For those tests no *.json-nft file is added. The bugs > needs > to be fixed first." > > Do you have a list of tests that are failing? Or maybe include this > list in the commit description? To keep them in the radar, we can > incrementally fix them. You can find them by running ./tools/check-tree.sh, it will print warnings about the tests that lack the .json-nft file. It's exactly those. It prints for example: WARN: "tests/shell/testcases/cache/0010_implicit_chain_0" has a dump file "tests/shell/testcases/cache/dumps/0010_implicit_chain_0.nft" but lacks a JSON dump "tests/shell/testcases/cache/dumps/0010_implicit_chain_0.json-nft" Thomas