All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nft] tests: 0043concatenated_ranges_0: Fix checks for add/delete failures
@ 2020-08-03 14:06 Stefano Brivio
  2020-08-04 11:44 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2020-08-03 14:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Florian Westphal

The test won't stop if we simply precede commands expected to fail
by !. POSIX.1-2017 says:

  -e
      When this option is on, if a simple command fails for any of
      the reasons listed in Consequences of Shell Errors or returns
      an exit status value >0, and is not part of the compound list
      following a while, until or if keyword, and is not a part of
      an AND or OR list, and is not a pipeline preceded by the "!"
      reserved word, then the shell will immediately exit.

...but I didn't care about the last part.

Replace those '! nft ...' commands by 'nft ... && exit 1' to actually
detect failures.

As a result, I didn't notice that now, correctly, inserting elements
into a set that contains the same exact element doesn't actually
fail, because nft doesn't pass NLM_F_EXCL on a simple 'add'. Drop
re-insertions from the checks we perform here, overlapping elements
are already covered by other tests.

Fixes: 618393c6b3f2 ("tests: Introduce test for set with concatenated ranges")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 .../testcases/sets/0043concatenated_ranges_0  | 27 +++++++------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/tests/shell/testcases/sets/0043concatenated_ranges_0 b/tests/shell/testcases/sets/0043concatenated_ranges_0
index 408040291aa8..11767373bcd2 100755
--- a/tests/shell/testcases/sets/0043concatenated_ranges_0
+++ b/tests/shell/testcases/sets/0043concatenated_ranges_0
@@ -6,11 +6,10 @@
 # all possible permutations, and:
 # - add entries to set
 # - list them
-# - check that they can't be added again
 # - get entries by specifying a value matching ranges for all fields
 # - delete them
+# - check that they can't be deleted again
 # - add them with 1s timeout
-# - check that they can't be added again right away
 # - check that they are not listed after 1s, just once, for the first entry
 # - delete them
 # - make sure they can't be deleted again
@@ -138,17 +137,11 @@ for ta in ${TYPES}; do
 				   -e "${a2} . ${b2} . ${c2}${mapv}"		\
 				   -e "${a3} . ${b3} . ${c3}${mapv}") -eq 3 ]
 
-			! ${NFT} "add element inet filter test \
-				  { ${a1} . ${b1} . ${c1}${mapv} };
-				  add element inet filter test \
-				  { ${a2} . ${b2} . ${c2}${mapv} };
-				  add element inet filter test \
-				  { ${a3} . ${b3} . ${c3}${mapv} }" 2>/dev/null
-
 			${NFT} delete element inet filter test \
 				"{ ${a1} . ${b1} . ${c1}${mapv} }"
-			! ${NFT} delete element inet filter test \
-				"{ ${a1} . ${b1} . ${c1}${mapv} }" 2>/dev/null
+			${NFT} delete element inet filter test \
+				"{ ${a1} . ${b1} . ${c1}${mapv} }" \
+				2>/dev/null && exit 1
 
 			eval add_a=\$ADD_${ta}
 			eval add_b=\$ADD_${tb}
@@ -157,9 +150,6 @@ for ta in ${TYPES}; do
 				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}"
 			[ $(${NFT} list ${setmap} inet filter test |	\
 			   grep -c "${add_a} . ${add_b} . ${add_c}") -eq 1 ]
-			! ${NFT} add element inet filter test \
-				"{ ${add_a} . ${add_b} . ${add_c} timeout 1s${mapv}}" \
-				2>/dev/null
 
 			eval get_a=\$GET_${ta}
 			eval get_b=\$GET_${tb}
@@ -175,17 +165,18 @@ for ta in ${TYPES}; do
 				{ ${a2} . ${b2} . ${c2}${mapv} };
 				delete element inet filter test \
 				{ ${a3} . ${b3} . ${c3}${mapv} }"
-			! ${NFT} "delete element inet filter test \
+			${NFT} "delete element inet filter test \
 				  { ${a2} . ${b2} . ${c2}${mapv} };
 				  delete element inet filter test \
-				  { ${a3} . ${b3} . ${c3} ${mapv} }" 2>/dev/null
+				  { ${a3} . ${b3} . ${c3} ${mapv} }" \
+				  2>/dev/null && exit 1
 
 			if [ ${timeout_tested} -eq 1 ]; then
 				${NFT} delete element inet filter test \
 					"{ ${add_a} . ${add_b} . ${add_c} ${mapv} }"
-				! ${NFT} delete element inet filter test \
+				${NFT} delete element inet filter test \
 					"{ ${add_a} . ${add_b} . ${add_c} ${mapv} }" \
-					2>/dev/null
+					2>/dev/null && exit 1
 				continue
 			fi
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH nft] tests: 0043concatenated_ranges_0: Fix checks for add/delete failures
  2020-08-03 14:06 [PATCH nft] tests: 0043concatenated_ranges_0: Fix checks for add/delete failures Stefano Brivio
@ 2020-08-04 11:44 ` Pablo Neira Ayuso
  0 siblings, 0 replies; 2+ messages in thread
From: Pablo Neira Ayuso @ 2020-08-04 11:44 UTC (permalink / raw)
  To: Stefano Brivio; +Cc: netfilter-devel, Florian Westphal

On Mon, Aug 03, 2020 at 04:06:39PM +0200, Stefano Brivio wrote:
> The test won't stop if we simply precede commands expected to fail
> by !. POSIX.1-2017 says:
> 
>   -e
>       When this option is on, if a simple command fails for any of
>       the reasons listed in Consequences of Shell Errors or returns
>       an exit status value >0, and is not part of the compound list
>       following a while, until or if keyword, and is not a part of
>       an AND or OR list, and is not a pipeline preceded by the "!"
>       reserved word, then the shell will immediately exit.
> 
> ...but I didn't care about the last part.
> 
> Replace those '! nft ...' commands by 'nft ... && exit 1' to actually
> detect failures.
> 
> As a result, I didn't notice that now, correctly, inserting elements
> into a set that contains the same exact element doesn't actually
> fail, because nft doesn't pass NLM_F_EXCL on a simple 'add'. Drop
> re-insertions from the checks we perform here, overlapping elements
> are already covered by other tests.

Also applied, thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-08-04 11:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-03 14:06 [PATCH nft] tests: 0043concatenated_ranges_0: Fix checks for add/delete failures Stefano Brivio
2020-08-04 11:44 ` Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.