Re: [PATCH] tests: shell: Add a test case to verify the limit statement.

[Florian Westphal <fw@strlen.de>]

Yi Chen <yiche@redhat.com> wrote:
> Signed-off-by: Yi Chen <yiche@redhat.com>
> ---
>  tests/shell/features/ncat.sh                |   4 +
>  tests/shell/testcases/packetpath/rate_limit | 154 ++++++++++++++++++++
>  2 files changed, 158 insertions(+)
>  create mode 100755 tests/shell/features/ncat.sh
>  create mode 100755 tests/shell/testcases/packetpath/rate_limit
> 
> diff --git a/tests/shell/features/ncat.sh b/tests/shell/features/ncat.sh
> new file mode 100755
> index 00000000..eb1581ce
> --- /dev/null
> +++ b/tests/shell/features/ncat.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +
> +# check whether ncat is installed
> +ncat -h >/dev/null 2>&1

Could you convert the test to use socat, which is already
used by many other tests?

> +assert_pass()
> +{
> +	local ret=$?
> +	if [ $ret != 0 ]; then
> +		echo "FAIL: ${@}"
> +		exit 1
> +	else
> +		echo "PASS: ${@}"
> +	fi
> +}

This is now the 3rd copy of this helper.

Maybe its time to add a library/utility file that has these
functions?

test-wrapper.sh could export an environment variable pointing to it, e.g.

export NFT_TEST_LIBRARY_FILE="$NFT_TEST_BASEDIR/helpers/lib.sh"

or whatever.  Then tests can do

. $NFT_TEST_LIBRARY_FILE

and gain these helpers in case they need them.

> +# tcp test
> +ip netns exec $S ncat -lk 80 > /dev/null & sleep 1

I guess sleep 1 is fine. But maybe its time to add a central
helper for this wait.

You could submit a patch that adds the helpers/lib.sh file and
then lifts the "wait_local_port_listen" helper function from

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/diff/tools/testing/selftests/net/lib.sh?id=d9d836bfa5e6e255c411733b4b1ce7a1f8346c54

Other than that I think the test looks good, thanks Yi!