On 2022-02-14, at 11:01:13 +0100, Phil Sutter wrote: > On Sat, Feb 12, 2022 at 04:58:30PM +0000, Jeremy Sowden wrote: > > Currently, there are two supported test results: `OK` and `FAIL`. > > It is expected that either the iptables command fails, or it > > succeeds and dumping the rule has the correct output. However, it > > is possible that the command may succeed but the output may not be > > correct. Add a `NOMATCH` result to cover this outcome. > > Hmm. Wouldn't it make sense to extend the scope of LEGACY/NFT keywords > to output checks as well instead of introducing a new one? I think we > could cover expected output that way by duplicating the test case with > different expected output instead of marking it as unspecific "may > produce garbage". Something like the following? One reason why I went with the `NOMATCH` result is that in the two divergent test-cases, there is no -nft output to match. We can make that work by just using the empty string as the alternative output since that will match anything. I don't think it's ideal, but it's simpler than overhauling the matching code for what is a rare corner case. J.
From a18fa07425f82d71a46846e4f3656ec65155fd0c Mon Sep 17 00:00:00 2001
From: Jeremy Sowden <jeremy@xxxxxxxxxx>
Date: Sun, 20 Feb 2022 12:49:23 +0000
Subject: [iptables PATCH v2] tests: specify non-matching output instead of
`NOMATCH` test result
Recently, we introduced a new test result `NOMATCH` to cover the case
where the test succeeds for both -legacy and -nft, but the two variants
do not have the same output. This patch does away with the new result
in favour of specifying the alternative output. For example, instead of
-j EXAMPLE-TARGET --example-option;=;OK;LEGACY;NOMATCH
which specifies that the test passes for -legacy with the output
matching the rule, but for -nft it passes with output which does not
match the rule, we specify the test as:
-j EXAMPLE-TARGET --example-option;=;OK;LEGACY;-j EXAMPLE-TARGET --nft-option
which specifies that the test passes for -legacy with the output
matching the rule, but for -nft it passes with the different output
given in the last field.
In the case of tests which have no output to match, we leave the last
field empty:
-j EXAMPLE-TARGET --example-option;=;OK;LEGACY;
Signed-off-by: Jeremy Sowden <jeremy@xxxxxxxxxx>
---
extensions/libxt_NFLOG.t | 4 ++--
iptables-test.py | 50 ++++++++++++++++++++++++++++++----------
2 files changed, 40 insertions(+), 14 deletions(-)
diff --git a/extensions/libxt_NFLOG.t b/extensions/libxt_NFLOG.t
index 25f332ae16b6..b3241f51b153 100644
--- a/extensions/libxt_NFLOG.t
+++ b/extensions/libxt_NFLOG.t
@@ -5,8 +5,8 @@
-j NFLOG --nflog-group 0;-j NFLOG;OK
# `--nflog-range` is broken and only supported by xtables-legacy.
# It has been superseded by `--nflog--group`.
--j NFLOG --nflog-range 1;=;OK;LEGACY;NOMATCH
--j NFLOG --nflog-range 4294967295;=;OK;LEGACY;NOMATCH
+-j NFLOG --nflog-range 1;=;OK;LEGACY;
+-j NFLOG --nflog-range 4294967295;=;OK;LEGACY;
-j NFLOG --nflog-range 4294967296;;FAIL
-j NFLOG --nflog-range -1;;FAIL
-j NFLOG --nflog-size 0;=;OK
diff --git a/iptables-test.py b/iptables-test.py
index 6acaa82228fa..763e5b449ca5 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -192,17 +192,18 @@ def execute_cmd(cmd, filename, lineno):
return ret
-def variant_res(res, variant, alt_res=None):
+def variant_res(res, variant, alt_rule_save):
'''
Adjust expected result with given variant
If expected result is scoped to a variant, the other one yields a different
- result. Therefore map @res to itself if given variant is current, use the
- alternate result, @alt_res, if specified, invert @res otherwise.
+ result or different output. Therefore map @res to itself if given variant is
+ current, use "OK" if the expected result is "OK" but the other variant yields
+ different output, invert @res otherwise.
- :param res: expected result from test spec ("OK", "FAIL" or "NOMATCH")
+ :param res: expected result from test spec ("OK" or "FAIL")
:param variant: variant @res is scoped to by test spec ("NFT" or "LEGACY")
- :param alt_res: optional expected result for the alternate variant.
+ :param alt_rule_save: alternative output if the variants have different output
'''
variant_executable = {
"NFT": "xtables-nft-multi",
@@ -210,17 +211,40 @@ def variant_res(res, variant, alt_res=None):
}
res_inverse = {
"OK": "FAIL",
- "FAIL": "OK",
- "NOMATCH": "OK"
+ "FAIL": "OK"
}
if variant_executable[variant] == EXECUTABLE:
return res
- if alt_res is not None:
- return alt_res
+ if res == "OK" and alt_rule_save is not None:
+ return res
return res_inverse[res]
+def variant_rule_save(rule_save, variant, alt_rule_save):
+ '''
+ Adjust expected output with given variant
+
+ If expected output is scoped to a variant, the other one yields different
+ output. Therefore map @rule_save to itself if given variant is current, use the
+ alternate output, @alt_rule_save otherwise.
+
+ :param rule_save: expected output if variant matches test spec
+ :param variant: variant given in test spec
+ :param alt_rule_save: expected output if the variant does not match the
+ test spec
+ '''
+ variant_executable = {
+ "NFT": "xtables-nft-multi",
+ "LEGACY": "xtables-legacy-multi"
+ }
+
+ if variant_executable[variant] == EXECUTABLE:
+ return rule_save
+ else:
+ return alt_rule_save
+
+
def run_test_file(filename, netns):
'''
Runs a test file
@@ -317,10 +341,12 @@ def run_test_file(filename, netns):
if len(item) > 3:
variant = item[3].rstrip()
if len(item) > 4:
- alt_res = item[4].rstrip()
+ alt_rule_save = item[4].rstrip()
else:
- alt_res = None
- res = variant_res(res, variant, alt_res)
+ alt_rule_save = None
+
+ res = variant_res(res, variant, alt_rule_save)
+ rule_save = variant_rule_save(rule_save, variant, alt_rule_save)
ret = run_test(iptables, rule, rule_save,
res, filename, lineno + 1, netns)
--
2.34.1
Attachment:
signature.asc
Description: PGP signature
