* [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0
@ 2023-09-27 16:39 Pablo Neira Ayuso
2023-09-28 14:58 ` Phil Sutter
0 siblings, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2023-09-27 16:39 UTC (permalink / raw)
To: netfilter-devel; +Cc: phil
A number of changes to fix spurious errors:
- Add seconds as expiration, otherwise 14m59 reports 14m in minute
granularity, this ensures suficient time in a very slow environment with
debugging instrumentation.
- Provide expected output.
- Update sed regular expression to make 'ms' optional and use -E mode.
Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ]
- Make 'ms' optional in sed regular expression
- Use -E in sed
.../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0
index 12f10074409f..0fd016e9f857 100755
--- a/tests/shell/testcases/sets/0036add_set_element_expiration_0
+++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0
@@ -3,17 +3,21 @@
set -e
drop_seconds() {
- sed 's/m[0-9]*s[0-9]*ms/m/g'
+ sed -E 's/m[0-9]*s([0-9]*ms)?/m/g'
}
RULESET="add table ip x
+add set ip x y { type ipv4_addr; flags dynamic,timeout; }
+add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }"
+
+EXPECTED="add table ip x
add set ip x y { type ipv4_addr; flags dynamic,timeout; }
add element ip x y { 1.1.1.1 timeout 30m expires 15m }"
test_output=$($NFT -e -f - <<< "$RULESET" 2>&1 | grep -v '# new generation' | drop_seconds)
-if [ "$test_output" != "$RULESET" ] ; then
- $DIFF -u <(echo "$test_output") <(echo "$RULESET")
+if [ "$test_output" != "$EXPECTED" ] ; then
+ $DIFF -u <(echo "$test_output") <(echo "$EXPECTED")
exit 1
fi
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-27 16:39 [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 Pablo Neira Ayuso @ 2023-09-28 14:58 ` Phil Sutter 2023-09-28 19:17 ` Pablo Neira Ayuso 0 siblings, 1 reply; 7+ messages in thread From: Phil Sutter @ 2023-09-28 14:58 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel On Wed, Sep 27, 2023 at 06:39:37PM +0200, Pablo Neira Ayuso wrote: > A number of changes to fix spurious errors: > > - Add seconds as expiration, otherwise 14m59 reports 14m in minute > granularity, this ensures suficient time in a very slow environment with > debugging instrumentation. > > - Provide expected output. > > - Update sed regular expression to make 'ms' optional and use -E mode. > > Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0") > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ] > - Make 'ms' optional in sed regular expression > - Use -E in sed > > .../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > index 12f10074409f..0fd016e9f857 100755 > --- a/tests/shell/testcases/sets/0036add_set_element_expiration_0 > +++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > @@ -3,17 +3,21 @@ > set -e > > drop_seconds() { > - sed 's/m[0-9]*s[0-9]*ms/m/g' > + sed -E 's/m[0-9]*s([0-9]*ms)?/m/g' > } So sometimes there's no ms part in output. In theory one would have to make the seconds part optional, too. Funny how tedious these little things may become to fix. Anyway, it should work without -E by escaping braces and the question mark. But accoring to sed(1), -E is in POSIX meanwhile so no big deal. > RULESET="add table ip x > +add set ip x y { type ipv4_addr; flags dynamic,timeout; } > +add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }" > + > +EXPECTED="add table ip x > add set ip x y { type ipv4_addr; flags dynamic,timeout; } > add element ip x y { 1.1.1.1 timeout 30m expires 15m }" I would have piped RULESET through drop_seconds in the $DIFF call below, but this variant surely saves a few cycles. :D > test_output=$($NFT -e -f - <<< "$RULESET" 2>&1 | grep -v '# new generation' | drop_seconds) > > -if [ "$test_output" != "$RULESET" ] ; then > - $DIFF -u <(echo "$test_output") <(echo "$RULESET") > +if [ "$test_output" != "$EXPECTED" ] ; then > + $DIFF -u <(echo "$test_output") <(echo "$EXPECTED") > exit 1 > fi Cheers, Phil ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-28 14:58 ` Phil Sutter @ 2023-09-28 19:17 ` Pablo Neira Ayuso 2023-09-29 9:49 ` Phil Sutter 0 siblings, 1 reply; 7+ messages in thread From: Pablo Neira Ayuso @ 2023-09-28 19:17 UTC (permalink / raw) To: Phil Sutter, netfilter-devel On Thu, Sep 28, 2023 at 04:58:46PM +0200, Phil Sutter wrote: > On Wed, Sep 27, 2023 at 06:39:37PM +0200, Pablo Neira Ayuso wrote: > > A number of changes to fix spurious errors: > > > > - Add seconds as expiration, otherwise 14m59 reports 14m in minute > > granularity, this ensures suficient time in a very slow environment with > > debugging instrumentation. > > > > - Provide expected output. > > > > - Update sed regular expression to make 'ms' optional and use -E mode. > > > > Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0") > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > --- > > v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ] > > - Make 'ms' optional in sed regular expression > > - Use -E in sed > > > > .../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++--- > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > index 12f10074409f..0fd016e9f857 100755 > > --- a/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > +++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > @@ -3,17 +3,21 @@ > > set -e > > > > drop_seconds() { > > - sed 's/m[0-9]*s[0-9]*ms/m/g' > > + sed -E 's/m[0-9]*s([0-9]*ms)?/m/g' > > } > > So sometimes there's no ms part in output. In theory one would have to > make the seconds part optional, too. Funny how tedious these little > things may become to fix. > > Anyway, it should work without -E by escaping braces and the question > mark. But accoring to sed(1), -E is in POSIX meanwhile so no big deal. > > > RULESET="add table ip x > > +add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > +add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }" > > + > > +EXPECTED="add table ip x > > add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > add element ip x y { 1.1.1.1 timeout 30m expires 15m }" > > I would have piped RULESET through drop_seconds in the $DIFF call below, > but this variant surely saves a few cycles. :D I am useless. I quickly tried this, but echo disregards newlines, triggering a mismatch in the diff, perhaps printf can address this. I just wanted to make this reliable, it has been bothering me in my test infrastructure which is running this in a loop and occasionally triggering this false positive. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-28 19:17 ` Pablo Neira Ayuso @ 2023-09-29 9:49 ` Phil Sutter 2023-09-29 9:53 ` Pablo Neira Ayuso 0 siblings, 1 reply; 7+ messages in thread From: Phil Sutter @ 2023-09-29 9:49 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel On Thu, Sep 28, 2023 at 09:17:20PM +0200, Pablo Neira Ayuso wrote: > On Thu, Sep 28, 2023 at 04:58:46PM +0200, Phil Sutter wrote: > > On Wed, Sep 27, 2023 at 06:39:37PM +0200, Pablo Neira Ayuso wrote: > > > A number of changes to fix spurious errors: > > > > > > - Add seconds as expiration, otherwise 14m59 reports 14m in minute > > > granularity, this ensures suficient time in a very slow environment with > > > debugging instrumentation. > > > > > > - Provide expected output. > > > > > > - Update sed regular expression to make 'ms' optional and use -E mode. > > > > > > Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0") > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > --- > > > v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ] > > > - Make 'ms' optional in sed regular expression > > > - Use -E in sed > > > > > > .../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++--- > > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > > > diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > index 12f10074409f..0fd016e9f857 100755 > > > --- a/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > +++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > @@ -3,17 +3,21 @@ > > > set -e > > > > > > drop_seconds() { > > > - sed 's/m[0-9]*s[0-9]*ms/m/g' > > > + sed -E 's/m[0-9]*s([0-9]*ms)?/m/g' > > > } > > > > So sometimes there's no ms part in output. In theory one would have to > > make the seconds part optional, too. Funny how tedious these little > > things may become to fix. > > > > Anyway, it should work without -E by escaping braces and the question > > mark. But accoring to sed(1), -E is in POSIX meanwhile so no big deal. > > > > > RULESET="add table ip x > > > +add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > +add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }" > > > + > > > +EXPECTED="add table ip x > > > add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > add element ip x y { 1.1.1.1 timeout 30m expires 15m }" > > > > I would have piped RULESET through drop_seconds in the $DIFF call below, > > but this variant surely saves a few cycles. :D > > I am useless. I quickly tried this, but echo disregards newlines, > triggering a mismatch in the diff, perhaps printf can address this. You probably forgot to quote the variable: | $ a="foo | > bar" | $ echo $a | foo bar | $ echo "$a" | foo | bar The following works fine for me: | RULESET=$(drop_seconds <<< "$RULESET") > I just wanted to make this reliable, it has been bothering me in my > test infrastructure which is running this in a loop and occasionally > triggering this false positive. ACK. These things must get fixed, otherwise running the testsuite before submitting/pushing becomes useless ("it fails anyway"). Cheers, Phil PS: I found this works fine: | drop_seconds() { | sed 's/[0-9]\+m\?s//g' | } So just "drop all occurences of numbers followed by ms or s". You have to send a follow-up anyway for the s/15m/14m59s/ change, right? PPS: The 'exit 1' is pointless: The script has 'set -e' and $DIFF returns non-zero if input differs. ;) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-29 9:49 ` Phil Sutter @ 2023-09-29 9:53 ` Pablo Neira Ayuso 2023-09-29 10:37 ` Phil Sutter 0 siblings, 1 reply; 7+ messages in thread From: Pablo Neira Ayuso @ 2023-09-29 9:53 UTC (permalink / raw) To: Phil Sutter, netfilter-devel On Fri, Sep 29, 2023 at 11:49:46AM +0200, Phil Sutter wrote: > On Thu, Sep 28, 2023 at 09:17:20PM +0200, Pablo Neira Ayuso wrote: > > On Thu, Sep 28, 2023 at 04:58:46PM +0200, Phil Sutter wrote: > > > On Wed, Sep 27, 2023 at 06:39:37PM +0200, Pablo Neira Ayuso wrote: > > > > A number of changes to fix spurious errors: > > > > > > > > - Add seconds as expiration, otherwise 14m59 reports 14m in minute > > > > granularity, this ensures suficient time in a very slow environment with > > > > debugging instrumentation. > > > > > > > > - Provide expected output. > > > > > > > > - Update sed regular expression to make 'ms' optional and use -E mode. > > > > > > > > Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0") > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > > --- > > > > v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ] > > > > - Make 'ms' optional in sed regular expression > > > > - Use -E in sed > > > > > > > > .../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++--- > > > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > index 12f10074409f..0fd016e9f857 100755 > > > > --- a/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > +++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > @@ -3,17 +3,21 @@ > > > > set -e > > > > > > > > drop_seconds() { > > > > - sed 's/m[0-9]*s[0-9]*ms/m/g' > > > > + sed -E 's/m[0-9]*s([0-9]*ms)?/m/g' > > > > } > > > > > > So sometimes there's no ms part in output. In theory one would have to > > > make the seconds part optional, too. Funny how tedious these little > > > things may become to fix. > > > > > > Anyway, it should work without -E by escaping braces and the question > > > mark. But accoring to sed(1), -E is in POSIX meanwhile so no big deal. > > > > > > > RULESET="add table ip x > > > > +add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > > +add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }" > > > > + > > > > +EXPECTED="add table ip x > > > > add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > > add element ip x y { 1.1.1.1 timeout 30m expires 15m }" > > > > > > I would have piped RULESET through drop_seconds in the $DIFF call below, > > > but this variant surely saves a few cycles. :D > > > > I am useless. I quickly tried this, but echo disregards newlines, > > triggering a mismatch in the diff, perhaps printf can address this. > > You probably forgot to quote the variable: > > | $ a="foo > | > bar" > | $ echo $a > | foo bar > | $ echo "$a" > | foo > | bar > > The following works fine for me: > > | RULESET=$(drop_seconds <<< "$RULESET") > > > I just wanted to make this reliable, it has been bothering me in my > > test infrastructure which is running this in a loop and occasionally > > triggering this false positive. > > ACK. These things must get fixed, otherwise running the testsuite before > submitting/pushing becomes useless ("it fails anyway"). > > Cheers, Phil > > PS: I found this works fine: > | drop_seconds() { > | sed 's/[0-9]\+m\?s//g' > | } > So just "drop all occurences of numbers followed by ms or s". > You have to send a follow-up anyway for the s/15m/14m59s/ change, > right? > > PPS: The 'exit 1' is pointless: The script has 'set -e' and $DIFF > returns non-zero if input differs. ;) Just follow up to refine if you feel like, but promise me this will not barf spuriously again :). ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-29 9:53 ` Pablo Neira Ayuso @ 2023-09-29 10:37 ` Phil Sutter 2023-09-29 10:54 ` Pablo Neira Ayuso 0 siblings, 1 reply; 7+ messages in thread From: Phil Sutter @ 2023-09-29 10:37 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netfilter-devel On Fri, Sep 29, 2023 at 11:53:33AM +0200, Pablo Neira Ayuso wrote: > On Fri, Sep 29, 2023 at 11:49:46AM +0200, Phil Sutter wrote: > > On Thu, Sep 28, 2023 at 09:17:20PM +0200, Pablo Neira Ayuso wrote: > > > On Thu, Sep 28, 2023 at 04:58:46PM +0200, Phil Sutter wrote: > > > > On Wed, Sep 27, 2023 at 06:39:37PM +0200, Pablo Neira Ayuso wrote: > > > > > A number of changes to fix spurious errors: > > > > > > > > > > - Add seconds as expiration, otherwise 14m59 reports 14m in minute > > > > > granularity, this ensures suficient time in a very slow environment with > > > > > debugging instrumentation. > > > > > > > > > > - Provide expected output. > > > > > > > > > > - Update sed regular expression to make 'ms' optional and use -E mode. > > > > > > > > > > Fixes: adf38fd84257 ("tests: shell: use minutes granularity in sets/0036add_set_element_expiration_0") > > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > > > --- > > > > > v3: - [ "$test_output" != "$EXPECTED" ], not [ "$test_output" != "$RULESET" ] > > > > > - Make 'ms' optional in sed regular expression > > > > > - Use -E in sed > > > > > > > > > > .../testcases/sets/0036add_set_element_expiration_0 | 10 +++++++--- > > > > > 1 file changed, 7 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/tests/shell/testcases/sets/0036add_set_element_expiration_0 b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > > index 12f10074409f..0fd016e9f857 100755 > > > > > --- a/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > > +++ b/tests/shell/testcases/sets/0036add_set_element_expiration_0 > > > > > @@ -3,17 +3,21 @@ > > > > > set -e > > > > > > > > > > drop_seconds() { > > > > > - sed 's/m[0-9]*s[0-9]*ms/m/g' > > > > > + sed -E 's/m[0-9]*s([0-9]*ms)?/m/g' > > > > > } > > > > > > > > So sometimes there's no ms part in output. In theory one would have to > > > > make the seconds part optional, too. Funny how tedious these little > > > > things may become to fix. > > > > > > > > Anyway, it should work without -E by escaping braces and the question > > > > mark. But accoring to sed(1), -E is in POSIX meanwhile so no big deal. > > > > > > > > > RULESET="add table ip x > > > > > +add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > > > +add element ip x y { 1.1.1.1 timeout 30m expires 15m59s }" > > > > > + > > > > > +EXPECTED="add table ip x > > > > > add set ip x y { type ipv4_addr; flags dynamic,timeout; } > > > > > add element ip x y { 1.1.1.1 timeout 30m expires 15m }" > > > > > > > > I would have piped RULESET through drop_seconds in the $DIFF call below, > > > > but this variant surely saves a few cycles. :D > > > > > > I am useless. I quickly tried this, but echo disregards newlines, > > > triggering a mismatch in the diff, perhaps printf can address this. > > > > You probably forgot to quote the variable: > > > > | $ a="foo > > | > bar" > > | $ echo $a > > | foo bar > > | $ echo "$a" > > | foo > > | bar > > > > The following works fine for me: > > > > | RULESET=$(drop_seconds <<< "$RULESET") > > > > > I just wanted to make this reliable, it has been bothering me in my > > > test infrastructure which is running this in a loop and occasionally > > > triggering this false positive. > > > > ACK. These things must get fixed, otherwise running the testsuite before > > submitting/pushing becomes useless ("it fails anyway"). > > > > Cheers, Phil > > > > PS: I found this works fine: > > | drop_seconds() { > > | sed 's/[0-9]\+m\?s//g' > > | } > > So just "drop all occurences of numbers followed by ms or s". > > You have to send a follow-up anyway for the s/15m/14m59s/ change, > > right? Scratch that, for some reason I just noticed "15m" in RULESET but missed that there's EXPECTED with "14m59s" (whic is piped through drop_seconds). > > PPS: The 'exit 1' is pointless: The script has 'set -e' and $DIFF > > returns non-zero if input differs. ;) > > Just follow up to refine if you feel like, but promise me this will > not barf spuriously again :). No need, I was merely nit-picking. If the test is stable now, it's entirely sufficient. Cheers, Phil ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 2023-09-29 10:37 ` Phil Sutter @ 2023-09-29 10:54 ` Pablo Neira Ayuso 0 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2023-09-29 10:54 UTC (permalink / raw) To: Phil Sutter, netfilter-devel On Fri, Sep 29, 2023 at 12:37:11PM +0200, Phil Sutter wrote: > No need, I was merely nit-picking. If the test is stable now, it's > entirely sufficient. Thanks, agreed. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-09-29 10:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-09-27 16:39 [PATCH nft,v3] tests: shell: fix spurious errors in sets/0036add_set_element_expiration_0 Pablo Neira Ayuso 2023-09-28 14:58 ` Phil Sutter 2023-09-28 19:17 ` Pablo Neira Ayuso 2023-09-29 9:49 ` Phil Sutter 2023-09-29 9:53 ` Pablo Neira Ayuso 2023-09-29 10:37 ` Phil Sutter 2023-09-29 10:54 ` Pablo Neira Ayuso
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).