* [nft PATCH v2] tests: shell: Review test-cases for destroy command
@ 2023-08-03 10:57 Phil Sutter
2023-08-03 12:52 ` Fernando F. Mancera
0 siblings, 1 reply; 3+ messages in thread
From: Phil Sutter @ 2023-08-03 10:57 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, Fernando F. Mancera
Having separate files for successful destroy of existing and
non-existing objects is a bit too much, just combine them into one.
While being at it:
* No bashisms, using /bin/sh is fine
* Append '-e' to shebang itself instead of calling 'set'
* Use 'nft -a -e' instead of assuming the created rule's handle value
* Shellcheck warned about curly braces, quote them
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Non-existence already asserted via dump files
- Drop dump files left-over after removing tests
---
tests/shell/testcases/chains/0044chain_destroy_0 | 11 +++++++----
tests/shell/testcases/chains/0045chain_destroy_0 | 8 --------
.../testcases/chains/dumps/0045chain_destroy_0.nft | 2 --
tests/shell/testcases/flowtable/0015destroy_0 | 9 ++++++---
tests/shell/testcases/flowtable/0016destroy_0 | 6 ------
.../shell/testcases/flowtable/dumps/0016destroy_0.nft | 2 --
tests/shell/testcases/maps/0014destroy_0 | 10 ++++++----
tests/shell/testcases/maps/0015destroy_0 | 7 -------
tests/shell/testcases/maps/dumps/0015destroy_0.nft | 2 --
tests/shell/testcases/rule_management/0012destroy_0 | 9 +++++++--
tests/shell/testcases/rule_management/0013destroy_0 | 8 --------
.../testcases/rule_management/dumps/0013destroy_0 | 4 ----
tests/shell/testcases/sets/0072destroy_0 | 10 ++++++----
tests/shell/testcases/sets/0073destroy_0 | 7 -------
tests/shell/testcases/sets/dumps/0073destroy_0.nft | 2 --
15 files changed, 32 insertions(+), 65 deletions(-)
delete mode 100755 tests/shell/testcases/chains/0045chain_destroy_0
delete mode 100644 tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
delete mode 100755 tests/shell/testcases/flowtable/0016destroy_0
delete mode 100644 tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
delete mode 100755 tests/shell/testcases/maps/0015destroy_0
delete mode 100644 tests/shell/testcases/maps/dumps/0015destroy_0.nft
delete mode 100755 tests/shell/testcases/rule_management/0013destroy_0
delete mode 100644 tests/shell/testcases/rule_management/dumps/0013destroy_0
delete mode 100755 tests/shell/testcases/sets/0073destroy_0
delete mode 100644 tests/shell/testcases/sets/dumps/0073destroy_0.nft
diff --git a/tests/shell/testcases/chains/0044chain_destroy_0 b/tests/shell/testcases/chains/0044chain_destroy_0
index 070021cf12f24..8384da66a5b0d 100755
--- a/tests/shell/testcases/chains/0044chain_destroy_0
+++ b/tests/shell/testcases/chains/0044chain_destroy_0
@@ -1,7 +1,10 @@
-#!/bin/bash
-
-set -e
+#!/bin/sh -e
$NFT add table t
-$NFT destroy chain t nochain
+# pass for non-existent chain
+$NFT destroy chain t c
+
+# successfully delete existing chain
+$NFT add chain t c
+$NFT destroy chain t c
diff --git a/tests/shell/testcases/chains/0045chain_destroy_0 b/tests/shell/testcases/chains/0045chain_destroy_0
deleted file mode 100755
index b356f8f885ca4..0000000000000
--- a/tests/shell/testcases/chains/0045chain_destroy_0
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/bash
-
-set -e
-
-$NFT add table t
-$NFT add chain t c
-
-$NFT destroy chain t c
diff --git a/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft b/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
deleted file mode 100644
index 985768baf3a1f..0000000000000
--- a/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
+++ /dev/null
@@ -1,2 +0,0 @@
-table ip t {
-}
diff --git a/tests/shell/testcases/flowtable/0015destroy_0 b/tests/shell/testcases/flowtable/0015destroy_0
index 4828d8187018c..66fce4992a502 100755
--- a/tests/shell/testcases/flowtable/0015destroy_0
+++ b/tests/shell/testcases/flowtable/0015destroy_0
@@ -1,7 +1,10 @@
-#!/bin/bash
+#!/bin/sh -e
-set -e
$NFT add table t
-$NFT add flowtable t f { hook ingress priority 10 \; devices = { lo }\; }
+# pass for non-existent flowtable
+$NFT destroy flowtable t f
+
+# successfully delete existing flowtable
+$NFT add flowtable t f '{ hook ingress priority 10; devices = { lo }; }'
$NFT destroy flowtable t f
diff --git a/tests/shell/testcases/flowtable/0016destroy_0 b/tests/shell/testcases/flowtable/0016destroy_0
deleted file mode 100755
index ce23c753365af..0000000000000
--- a/tests/shell/testcases/flowtable/0016destroy_0
+++ /dev/null
@@ -1,6 +0,0 @@
-#!/bin/bash
-
-set -e
-$NFT add table t
-
-$NFT destroy flowtable t f
diff --git a/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft b/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
deleted file mode 100644
index 985768baf3a1f..0000000000000
--- a/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
+++ /dev/null
@@ -1,2 +0,0 @@
-table ip t {
-}
diff --git a/tests/shell/testcases/maps/0014destroy_0 b/tests/shell/testcases/maps/0014destroy_0
index b769276d52599..14c3f78af7f19 100755
--- a/tests/shell/testcases/maps/0014destroy_0
+++ b/tests/shell/testcases/maps/0014destroy_0
@@ -1,8 +1,10 @@
-#!/bin/bash
-
-set -e
+#!/bin/sh -e
$NFT add table x
-$NFT add map x y { type ipv4_addr : ipv4_addr\; }
+# pass for non-existent map
+$NFT destroy map x y
+
+# successfully delete existing map
+$NFT add map x y '{ type ipv4_addr : ipv4_addr; }'
$NFT destroy map x y
diff --git a/tests/shell/testcases/maps/0015destroy_0 b/tests/shell/testcases/maps/0015destroy_0
deleted file mode 100755
index abad4d57e7221..0000000000000
--- a/tests/shell/testcases/maps/0015destroy_0
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/bin/bash
-
-set -e
-
-$NFT add table x
-
-$NFT destroy map x nonmap
diff --git a/tests/shell/testcases/maps/dumps/0015destroy_0.nft b/tests/shell/testcases/maps/dumps/0015destroy_0.nft
deleted file mode 100644
index 5d4d2cafb577d..0000000000000
--- a/tests/shell/testcases/maps/dumps/0015destroy_0.nft
+++ /dev/null
@@ -1,2 +0,0 @@
-table ip x {
-}
diff --git a/tests/shell/testcases/rule_management/0012destroy_0 b/tests/shell/testcases/rule_management/0012destroy_0
index 1b61155e9b7ef..85f9c9f6d4c78 100755
--- a/tests/shell/testcases/rule_management/0012destroy_0
+++ b/tests/shell/testcases/rule_management/0012destroy_0
@@ -1,7 +1,12 @@
-#!/bin/bash
+#!/bin/sh -e
-set -e
$NFT add table t
$NFT add chain t c
+# pass for non-existent rule
$NFT destroy rule t c handle 3333
+
+# successfully delete existing rule
+handle=$($NFT -a -e insert rule t c accept | \
+ sed -n 's/.*handle \([0-9]*\)$/\1/p')
+$NFT destroy rule t c handle "$handle"
diff --git a/tests/shell/testcases/rule_management/0013destroy_0 b/tests/shell/testcases/rule_management/0013destroy_0
deleted file mode 100755
index 895c24a4dacba..0000000000000
--- a/tests/shell/testcases/rule_management/0013destroy_0
+++ /dev/null
@@ -1,8 +0,0 @@
-#!/bin/bash
-
-set -e
-$NFT add table t
-$NFT add chain t c
-$NFT insert rule t c accept # should have handle 2
-
-$NFT destroy rule t c handle 2
diff --git a/tests/shell/testcases/rule_management/dumps/0013destroy_0 b/tests/shell/testcases/rule_management/dumps/0013destroy_0
deleted file mode 100644
index 1e0d1d603739b..0000000000000
--- a/tests/shell/testcases/rule_management/dumps/0013destroy_0
+++ /dev/null
@@ -1,4 +0,0 @@
-table ip t {
- chain c {
- }
-}
diff --git a/tests/shell/testcases/sets/0072destroy_0 b/tests/shell/testcases/sets/0072destroy_0
index c9cf9ff716349..fd1d645057c09 100755
--- a/tests/shell/testcases/sets/0072destroy_0
+++ b/tests/shell/testcases/sets/0072destroy_0
@@ -1,8 +1,10 @@
-#!/bin/bash
-
-set -e
+#!/bin/sh -e
$NFT add table x
-$NFT add set x s {type ipv4_addr\; size 2\;}
+# pass for non-existent set
+$NFT destroy set x s
+
+# successfully delete existing set
+$NFT add set x s '{type ipv4_addr; size 2;}'
$NFT destroy set x s
diff --git a/tests/shell/testcases/sets/0073destroy_0 b/tests/shell/testcases/sets/0073destroy_0
deleted file mode 100755
index a9d65a5541c53..0000000000000
--- a/tests/shell/testcases/sets/0073destroy_0
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/bin/bash
-
-set -e
-
-$NFT add table x
-
-$NFT destroy set x s
diff --git a/tests/shell/testcases/sets/dumps/0073destroy_0.nft b/tests/shell/testcases/sets/dumps/0073destroy_0.nft
deleted file mode 100644
index 5d4d2cafb577d..0000000000000
--- a/tests/shell/testcases/sets/dumps/0073destroy_0.nft
+++ /dev/null
@@ -1,2 +0,0 @@
-table ip x {
-}
--
2.40.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [nft PATCH v2] tests: shell: Review test-cases for destroy command
2023-08-03 10:57 [nft PATCH v2] tests: shell: Review test-cases for destroy command Phil Sutter
@ 2023-08-03 12:52 ` Fernando F. Mancera
2023-08-03 12:59 ` Phil Sutter
0 siblings, 1 reply; 3+ messages in thread
From: Fernando F. Mancera @ 2023-08-03 12:52 UTC (permalink / raw)
To: Phil Sutter, Pablo Neira Ayuso; +Cc: netfilter-devel
Looks good to me, thank you!
On 03/08/2023 12:57, Phil Sutter wrote:
> Having separate files for successful destroy of existing and
> non-existing objects is a bit too much, just combine them into one.
>
> While being at it:
>
> * No bashisms, using /bin/sh is fine
> * Append '-e' to shebang itself instead of calling 'set'
> * Use 'nft -a -e' instead of assuming the created rule's handle value
> * Shellcheck warned about curly braces, quote them
>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Non-existence already asserted via dump files
> - Drop dump files left-over after removing tests
> ---
> tests/shell/testcases/chains/0044chain_destroy_0 | 11 +++++++----
> tests/shell/testcases/chains/0045chain_destroy_0 | 8 --------
> .../testcases/chains/dumps/0045chain_destroy_0.nft | 2 --
> tests/shell/testcases/flowtable/0015destroy_0 | 9 ++++++---
> tests/shell/testcases/flowtable/0016destroy_0 | 6 ------
> .../shell/testcases/flowtable/dumps/0016destroy_0.nft | 2 --
> tests/shell/testcases/maps/0014destroy_0 | 10 ++++++----
> tests/shell/testcases/maps/0015destroy_0 | 7 -------
> tests/shell/testcases/maps/dumps/0015destroy_0.nft | 2 --
> tests/shell/testcases/rule_management/0012destroy_0 | 9 +++++++--
> tests/shell/testcases/rule_management/0013destroy_0 | 8 --------
> .../testcases/rule_management/dumps/0013destroy_0 | 4 ----
> tests/shell/testcases/sets/0072destroy_0 | 10 ++++++----
> tests/shell/testcases/sets/0073destroy_0 | 7 -------
> tests/shell/testcases/sets/dumps/0073destroy_0.nft | 2 --
> 15 files changed, 32 insertions(+), 65 deletions(-)
> delete mode 100755 tests/shell/testcases/chains/0045chain_destroy_0
> delete mode 100644 tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
> delete mode 100755 tests/shell/testcases/flowtable/0016destroy_0
> delete mode 100644 tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
> delete mode 100755 tests/shell/testcases/maps/0015destroy_0
> delete mode 100644 tests/shell/testcases/maps/dumps/0015destroy_0.nft
> delete mode 100755 tests/shell/testcases/rule_management/0013destroy_0
> delete mode 100644 tests/shell/testcases/rule_management/dumps/0013destroy_0
> delete mode 100755 tests/shell/testcases/sets/0073destroy_0
> delete mode 100644 tests/shell/testcases/sets/dumps/0073destroy_0.nft
>
> diff --git a/tests/shell/testcases/chains/0044chain_destroy_0 b/tests/shell/testcases/chains/0044chain_destroy_0
> index 070021cf12f24..8384da66a5b0d 100755
> --- a/tests/shell/testcases/chains/0044chain_destroy_0
> +++ b/tests/shell/testcases/chains/0044chain_destroy_0
> @@ -1,7 +1,10 @@
> -#!/bin/bash
> -
> -set -e
> +#!/bin/sh -e
>
> $NFT add table t
>
> -$NFT destroy chain t nochain
> +# pass for non-existent chain
> +$NFT destroy chain t c
> +
> +# successfully delete existing chain
> +$NFT add chain t c
> +$NFT destroy chain t c
> diff --git a/tests/shell/testcases/chains/0045chain_destroy_0 b/tests/shell/testcases/chains/0045chain_destroy_0
> deleted file mode 100755
> index b356f8f885ca4..0000000000000
> --- a/tests/shell/testcases/chains/0045chain_destroy_0
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -#!/bin/bash
> -
> -set -e
> -
> -$NFT add table t
> -$NFT add chain t c
> -
> -$NFT destroy chain t c
> diff --git a/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft b/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
> deleted file mode 100644
> index 985768baf3a1f..0000000000000
> --- a/tests/shell/testcases/chains/dumps/0045chain_destroy_0.nft
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -table ip t {
> -}
> diff --git a/tests/shell/testcases/flowtable/0015destroy_0 b/tests/shell/testcases/flowtable/0015destroy_0
> index 4828d8187018c..66fce4992a502 100755
> --- a/tests/shell/testcases/flowtable/0015destroy_0
> +++ b/tests/shell/testcases/flowtable/0015destroy_0
> @@ -1,7 +1,10 @@
> -#!/bin/bash
> +#!/bin/sh -e
>
> -set -e
> $NFT add table t
> -$NFT add flowtable t f { hook ingress priority 10 \; devices = { lo }\; }
>
> +# pass for non-existent flowtable
> +$NFT destroy flowtable t f
> +
> +# successfully delete existing flowtable
> +$NFT add flowtable t f '{ hook ingress priority 10; devices = { lo }; }'
> $NFT destroy flowtable t f
> diff --git a/tests/shell/testcases/flowtable/0016destroy_0 b/tests/shell/testcases/flowtable/0016destroy_0
> deleted file mode 100755
> index ce23c753365af..0000000000000
> --- a/tests/shell/testcases/flowtable/0016destroy_0
> +++ /dev/null
> @@ -1,6 +0,0 @@
> -#!/bin/bash
> -
> -set -e
> -$NFT add table t
> -
> -$NFT destroy flowtable t f
> diff --git a/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft b/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
> deleted file mode 100644
> index 985768baf3a1f..0000000000000
> --- a/tests/shell/testcases/flowtable/dumps/0016destroy_0.nft
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -table ip t {
> -}
> diff --git a/tests/shell/testcases/maps/0014destroy_0 b/tests/shell/testcases/maps/0014destroy_0
> index b769276d52599..14c3f78af7f19 100755
> --- a/tests/shell/testcases/maps/0014destroy_0
> +++ b/tests/shell/testcases/maps/0014destroy_0
> @@ -1,8 +1,10 @@
> -#!/bin/bash
> -
> -set -e
> +#!/bin/sh -e
>
> $NFT add table x
> -$NFT add map x y { type ipv4_addr : ipv4_addr\; }
>
> +# pass for non-existent map
> +$NFT destroy map x y
> +
> +# successfully delete existing map
> +$NFT add map x y '{ type ipv4_addr : ipv4_addr; }'
> $NFT destroy map x y
> diff --git a/tests/shell/testcases/maps/0015destroy_0 b/tests/shell/testcases/maps/0015destroy_0
> deleted file mode 100755
> index abad4d57e7221..0000000000000
> --- a/tests/shell/testcases/maps/0015destroy_0
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#!/bin/bash
> -
> -set -e
> -
> -$NFT add table x
> -
> -$NFT destroy map x nonmap
> diff --git a/tests/shell/testcases/maps/dumps/0015destroy_0.nft b/tests/shell/testcases/maps/dumps/0015destroy_0.nft
> deleted file mode 100644
> index 5d4d2cafb577d..0000000000000
> --- a/tests/shell/testcases/maps/dumps/0015destroy_0.nft
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -table ip x {
> -}
> diff --git a/tests/shell/testcases/rule_management/0012destroy_0 b/tests/shell/testcases/rule_management/0012destroy_0
> index 1b61155e9b7ef..85f9c9f6d4c78 100755
> --- a/tests/shell/testcases/rule_management/0012destroy_0
> +++ b/tests/shell/testcases/rule_management/0012destroy_0
> @@ -1,7 +1,12 @@
> -#!/bin/bash
> +#!/bin/sh -e
>
> -set -e
> $NFT add table t
> $NFT add chain t c
>
> +# pass for non-existent rule
> $NFT destroy rule t c handle 3333
> +
> +# successfully delete existing rule
> +handle=$($NFT -a -e insert rule t c accept | \
> + sed -n 's/.*handle \([0-9]*\)$/\1/p')
> +$NFT destroy rule t c handle "$handle"
> diff --git a/tests/shell/testcases/rule_management/0013destroy_0 b/tests/shell/testcases/rule_management/0013destroy_0
> deleted file mode 100755
> index 895c24a4dacba..0000000000000
> --- a/tests/shell/testcases/rule_management/0013destroy_0
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -#!/bin/bash
> -
> -set -e
> -$NFT add table t
> -$NFT add chain t c
> -$NFT insert rule t c accept # should have handle 2
> -
> -$NFT destroy rule t c handle 2
> diff --git a/tests/shell/testcases/rule_management/dumps/0013destroy_0 b/tests/shell/testcases/rule_management/dumps/0013destroy_0
> deleted file mode 100644
> index 1e0d1d603739b..0000000000000
> --- a/tests/shell/testcases/rule_management/dumps/0013destroy_0
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -table ip t {
> - chain c {
> - }
> -}
> diff --git a/tests/shell/testcases/sets/0072destroy_0 b/tests/shell/testcases/sets/0072destroy_0
> index c9cf9ff716349..fd1d645057c09 100755
> --- a/tests/shell/testcases/sets/0072destroy_0
> +++ b/tests/shell/testcases/sets/0072destroy_0
> @@ -1,8 +1,10 @@
> -#!/bin/bash
> -
> -set -e
> +#!/bin/sh -e
>
> $NFT add table x
> -$NFT add set x s {type ipv4_addr\; size 2\;}
>
> +# pass for non-existent set
> +$NFT destroy set x s
> +
> +# successfully delete existing set
> +$NFT add set x s '{type ipv4_addr; size 2;}'
> $NFT destroy set x s
> diff --git a/tests/shell/testcases/sets/0073destroy_0 b/tests/shell/testcases/sets/0073destroy_0
> deleted file mode 100755
> index a9d65a5541c53..0000000000000
> --- a/tests/shell/testcases/sets/0073destroy_0
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#!/bin/bash
> -
> -set -e
> -
> -$NFT add table x
> -
> -$NFT destroy set x s
> diff --git a/tests/shell/testcases/sets/dumps/0073destroy_0.nft b/tests/shell/testcases/sets/dumps/0073destroy_0.nft
> deleted file mode 100644
> index 5d4d2cafb577d..0000000000000
> --- a/tests/shell/testcases/sets/dumps/0073destroy_0.nft
> +++ /dev/null
> @@ -1,2 +0,0 @@
> -table ip x {
> -}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [nft PATCH v2] tests: shell: Review test-cases for destroy command
2023-08-03 12:52 ` Fernando F. Mancera
@ 2023-08-03 12:59 ` Phil Sutter
0 siblings, 0 replies; 3+ messages in thread
From: Phil Sutter @ 2023-08-03 12:59 UTC (permalink / raw)
To: Fernando F. Mancera; +Cc: Pablo Neira Ayuso, netfilter-devel
On Thu, Aug 03, 2023 at 02:52:11PM +0200, Fernando F. Mancera wrote:
> Looks good to me, thank you!
Patch applied, thanks for your review!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-08-03 12:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-03 10:57 [nft PATCH v2] tests: shell: Review test-cases for destroy command Phil Sutter
2023-08-03 12:52 ` Fernando F. Mancera
2023-08-03 12:59 ` Phil Sutter
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).