[LTP] [PATCH v5 0/5] shell: Introduce TST_TIMEOUT variable

[LTP] [PATCH v5 0/5] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

Hi,

hopefully the latest version.

Changes v4->v5:
* remove float related code (left from v3)
* remove "tst_test_cmds cut" check from tst_test.sh (there is check for
cut and other later => it should be probably removed as well, but as a
separate patch) (Cyril)
* remove unneeded IFS from test (Cyril)
* mention ceiling LTP_TIMEOUT_MUL in doc/user-guide.txt

Here is the diff:
diff --git doc/user-guide.txt doc/user-guide.txt
index 8913c3221..7f6334ec2 100644
--- doc/user-guide.txt
+++ doc/user-guide.txt
@@ -15,7 +15,7 @@ For running LTP network tests see `testcases/network/README.md`.
                           'n' or '0': never colorize.
 | 'LTP_TIMEOUT_MUL'     | Multiply timeout, must be number >= 1 (> 1 is useful for
                           slow machines to avoid unexpected timeout).
-                          Variable is also used in shell tests.
+                          Variable is also used in shell tests, but here ceiled to int.
 | 'PATH'                | It's required to addjust path:
                           `PATH="$PATH:$LTPROOT/testcases/bin"`
 | 'TMPDIR'              | Base directory for template directory, which is required by C tests
diff --git lib/newlib_tests/shell/test_timeout.sh lib/newlib_tests/shell/test_timeout.sh
index 7e296f9e9..2cbc66412 100755
--- lib/newlib_tests/shell/test_timeout.sh
+++ lib/newlib_tests/shell/test_timeout.sh
@@ -15,8 +15,6 @@ echo "Testing timeout in shell API"
 echo
 
 failed=0
-IFS="
-"
 for i in $DATA; do
 	file=$(echo $i | cut -d'|' -f1)
 	timeout=$(echo $i | cut -d'|' -f2)
diff --git testcases/lib/tst_test.sh testcases/lib/tst_test.sh
index 977ffd97e..8713c1cdd 100644
--- testcases/lib/tst_test.sh
+++ testcases/lib/tst_test.sh
@@ -392,7 +392,6 @@ _tst_setup_timer()
 	tst_is_num "$LTP_TIMEOUT_MUL" || tst_brk TCONF "$err ($LTP_TIMEOUT_MUL)"
 
 	if ! tst_is_int "$LTP_TIMEOUT_MUL"; then
-		tst_test_cmds cut
 		LTP_TIMEOUT_MUL=$(echo "$LTP_TIMEOUT_MUL" | cut -d. -f1)
 		LTP_TIMEOUT_MUL=$((LTP_TIMEOUT_MUL+1))
 		tst_res TINFO "ceiling LTP_TIMEOUT_MUL to $LTP_TIMEOUT_MUL"
@@ -403,13 +402,7 @@ _tst_setup_timer()
 		tst_brk TBROK "TST_TIMEOUT must be int >= 1! ($TST_TIMEOUT)"
 	fi
 
-	local sec
-	if [ "$is_float" ]; then
-		sec=`echo | awk '{printf("%d\n", '$TST_TIMEOUT' * '$LTP_TIMEOUT_MUL'+ 0.5)}'`
-	else
-		sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
-	fi
-
+	local sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
 	local h=$((sec / 3600))
 	local m=$((sec / 60 % 60))
 	local s=$((sec % 60))


Petr Vorel (5):
  shell: Add tst_is_num()
  shell: Introduce TST_TIMEOUT variable, add checks
  shell: Add timeout shell API tests
  memcg_stress_test.sh: use TST_TIMEOUT (replace LTP_TIMEOUT_MUL)
  net/if-mtu-change.sh: set TST_TIMEOUT

 doc/test-writing-guidelines.txt               | 14 ++++++--
 doc/user-guide.txt                            |  2 +-
 lib/newlib_tests/shell/test_timeout.sh        | 36 +++++++++++++++++++
 lib/newlib_tests/shell/timeout01.sh           | 13 +++++++
 lib/newlib_tests/shell/timeout02.sh           | 13 +++++++
 .../memcg/stress/memcg_stress_test.sh         |  2 +-
 testcases/lib/tst_test.sh                     | 32 +++++++++++++++--
 .../network/stress/interface/if-mtu-change.sh |  4 ++-
 8 files changed, 108 insertions(+), 8 deletions(-)
 create mode 100755 lib/newlib_tests/shell/test_timeout.sh
 create mode 100755 lib/newlib_tests/shell/timeout01.sh
 create mode 100755 lib/newlib_tests/shell/timeout02.sh

-- 
2.23.0

[LTP] [PATCH v5 1/5] shell: Add tst_is_num()

[Petr Vorel]

Using grep -E, which more portable than using awk or anything else.

Reviewed-by: Li Wang <liwang@redhat.com>
Reviewed-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/test-writing-guidelines.txt | 6 ++++++
 testcases/lib/tst_test.sh       | 7 ++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 6da87baa7..997272cbe 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2338,6 +2338,12 @@ Checking for integers
 tst_is_int "$FOO"
 -------------------------------------------------------------------------------
 
+Checking for integers and floating point numbers
+++++++++++++++++++++++++++++++++++++++++++++++++
+# returns zero if passed an integer or floating point number parameter, non-zero otherwise
+tst_is_num "$FOO"
+-------------------------------------------------------------------------------
+
 Obtaining random numbers
 ++++++++++++++++++++++++
 
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index e0b24c6b9..ca63745fd 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0-or-later
-# Copyright (c) Linux Test Project, 2014-2018
+# Copyright (c) Linux Test Project, 2014-2019
 # Author: Cyril Hrubis <chrubis@suse.cz>
 #
 # LTP test library for shell.
@@ -344,6 +344,11 @@ tst_is_int()
 	return $?
 }
 
+tst_is_num()
+{
+	echo "$1" | grep -Eq '^[-+]?[0-9]+\.?[0-9]*$'
+}
+
 tst_usage()
 {
 	if [ -n "$TST_USAGE" ]; then
-- 
2.23.0

[LTP] [PATCH v5 2/5] shell: Introduce TST_TIMEOUT variable, add checks

[Petr Vorel]

to unify shell API with C API.

TST_TIMEOUT should be set in tests only, it's equivalent
of tst_test->timeout in C API.

Add checks requiring both TST_TIMEOUT and LTP_TIMEOUT_MUL >= 1,
that allow to set TST_TIMEOUT lower than the default 300 sec
(might be useful for some case).

LTP_TIMEOUT_MUL can be float, but will be ceiled to int.
Using float would require awk/bc, which is unnecessary dependency
and code complication (we do not care that much if it's multiplied
precisely as far as the resulting timeout is never smaller than
the precise calculation).

Also added cut dependency to _tst_setup_timer(), but that's not
a problem as it was already required for shell API in tst_run().

Suggested-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de>
Reviewed-by: Li Wang <liwang@redhat.com>
Reviewed-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/test-writing-guidelines.txt |  8 ++++++--
 doc/user-guide.txt              |  2 +-
 testcases/lib/tst_test.sh       | 25 +++++++++++++++++++++++--
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 997272cbe..efff4d40c 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2109,8 +2109,8 @@ tst_run
 '$TST_TEST_DATA' can be used with '$TST_CNT'. If '$TST_TEST_DATA_IFS' not specified,
 space as default value is used. Of course, it's possible to use separate functions.
 
-2.3.2 Library variables
-^^^^^^^^^^^^^^^^^^^^^^^
+2.3.2 Library environment variables for shell
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Similarily to the C library various checks and preparations can be requested
 simply by setting right '$TST_NEEDS_FOO'.
@@ -2126,6 +2126,10 @@ simply by setting right '$TST_NEEDS_FOO'.
                        the test (see below).
 | 'TST_NEEDS_MODULE' | Test module name needed for the test (see below).
 | 'TST_NEEDS_DRIVERS'| Checks kernel drivers support for the test.
+| 'TST_TIMEOUT'      | Maximum timeout set for the test in sec. Must be int >= 1,
+                       or -1 (special value to disable timeout), default is 300.
+                       Variable is meant be set in tests, not by user.
+                       It's equivalent of `tst_test.timeout` in C.
 |=============================================================================
 
 NOTE: Network tests (see testcases/network/README.md) use additional variables
diff --git a/doc/user-guide.txt b/doc/user-guide.txt
index 8913c3221..7f6334ec2 100644
--- a/doc/user-guide.txt
+++ b/doc/user-guide.txt
@@ -15,7 +15,7 @@ For running LTP network tests see `testcases/network/README.md`.
                           'n' or '0': never colorize.
 | 'LTP_TIMEOUT_MUL'     | Multiply timeout, must be number >= 1 (> 1 is useful for
                           slow machines to avoid unexpected timeout).
-                          Variable is also used in shell tests.
+                          Variable is also used in shell tests, but here ceiled to int.
 | 'PATH'                | It's required to addjust path:
                           `PATH="$PATH:$LTPROOT/testcases/bin"`
 | 'TMPDIR'              | Base directory for template directory, which is required by C tests
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index ca63745fd..8713c1cdd 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -379,9 +379,30 @@ _tst_rescmp()
 
 _tst_setup_timer()
 {
+	TST_TIMEOUT=${TST_TIMEOUT:-300}
 	LTP_TIMEOUT_MUL=${LTP_TIMEOUT_MUL:-1}
 
-	local sec=$((300 * LTP_TIMEOUT_MUL))
+	if [ "$TST_TIMEOUT" = -1 ]; then
+		tst_res TINFO "Timeout per run is disabled"
+		return
+	fi
+
+	local err="LTP_TIMEOUT_MUL must be number >= 1!"
+
+	tst_is_num "$LTP_TIMEOUT_MUL" || tst_brk TCONF "$err ($LTP_TIMEOUT_MUL)"
+
+	if ! tst_is_int "$LTP_TIMEOUT_MUL"; then
+		LTP_TIMEOUT_MUL=$(echo "$LTP_TIMEOUT_MUL" | cut -d. -f1)
+		LTP_TIMEOUT_MUL=$((LTP_TIMEOUT_MUL+1))
+		tst_res TINFO "ceiling LTP_TIMEOUT_MUL to $LTP_TIMEOUT_MUL"
+	fi
+	[ "$LTP_TIMEOUT_MUL" -ge 1 ] || tst_brk TCONF "$err ($LTP_TIMEOUT_MUL)"
+
+	if ! tst_is_int "$TST_TIMEOUT" || [ "$TST_TIMEOUT" -lt 1 ]; then
+		tst_brk TBROK "TST_TIMEOUT must be int >= 1! ($TST_TIMEOUT)"
+	fi
+
+	local sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
 	local h=$((sec / 3600))
 	local m=$((sec / 60 % 60))
 	local s=$((sec % 60))
@@ -418,7 +439,7 @@ tst_run()
 			NEEDS_CMDS|NEEDS_MODULE|MODPATH|DATAROOT);;
 			NEEDS_DRIVERS|FS_TYPE|MNTPOINT|MNT_PARAMS);;
 			IPV6|IPVER|TEST_DATA|TEST_DATA_IFS);;
-			RETRY_FUNC|RETRY_FN_EXP_BACKOFF);;
+			RETRY_FUNC|RETRY_FN_EXP_BACKOFF|TIMEOUT);;
 			NET_MAX_PKT);;
 			*) tst_res TWARN "Reserved variable TST_$_tst_i used!";;
 			esac
-- 
2.23.0

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Petr Vorel]

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 lib/newlib_tests/shell/test_timeout.sh | 36 ++++++++++++++++++++++++++
 lib/newlib_tests/shell/timeout01.sh    | 13 ++++++++++
 lib/newlib_tests/shell/timeout02.sh    | 13 ++++++++++
 3 files changed, 62 insertions(+)
 create mode 100755 lib/newlib_tests/shell/test_timeout.sh
 create mode 100755 lib/newlib_tests/shell/timeout01.sh
 create mode 100755 lib/newlib_tests/shell/timeout02.sh

diff --git a/lib/newlib_tests/shell/test_timeout.sh b/lib/newlib_tests/shell/test_timeout.sh
new file mode 100755
index 000000000..2cbc66412
--- /dev/null
+++ b/lib/newlib_tests/shell/test_timeout.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+PATH="$(dirname $0)/../../../testcases/lib/:$PATH"
+
+DATA="
+timeout01.sh||0
+timeout02.sh||0
+timeout02.sh|foo|32
+timeout02.sh|2|0
+timeout02.sh|1.1|0
+timeout02.sh|-10|32
+"
+
+echo "Testing timeout in shell API"
+echo
+
+failed=0
+for i in $DATA; do
+	file=$(echo $i | cut -d'|' -f1)
+	timeout=$(echo $i | cut -d'|' -f2)
+	exp_exit=$(echo $i | cut -d'|' -f3)
+
+	echo "=== $test (LTP_TIMEOUT_MUL='$timeout') ==="
+	LTP_TIMEOUT_MUL=$timeout ./$file
+	ret=$?
+	if [ $ret -ne $exp_exit ]; then
+		echo "FAILED (exit code: $ret, expected $exp_exit)"
+		failed=$((failed+1))
+	else
+		echo "PASSED"
+	fi
+	echo
+done
+
+echo "Failed tests: $failed"
+exit $failed
diff --git a/lib/newlib_tests/shell/timeout01.sh b/lib/newlib_tests/shell/timeout01.sh
new file mode 100755
index 000000000..ab7428a2d
--- /dev/null
+++ b/lib/newlib_tests/shell/timeout01.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+TST_TESTFUNC=do_test
+
+TST_TIMEOUT=-1
+. tst_test.sh
+
+do_test()
+{
+	tst_res TPASS "timeout $TST_TIMEOUT set"
+}
+
+tst_run
diff --git a/lib/newlib_tests/shell/timeout02.sh b/lib/newlib_tests/shell/timeout02.sh
new file mode 100755
index 000000000..73af09125
--- /dev/null
+++ b/lib/newlib_tests/shell/timeout02.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+
+TST_TESTFUNC=do_test
+
+TST_TIMEOUT=2
+. tst_test.sh
+
+do_test()
+{
+	tst_res TPASS "timeout $TST_TIMEOUT set (LTP_TIMEOUT_MUL='$LTP_TIMEOUT_MUL')"
+}
+
+tst_run
-- 
2.23.0

[LTP] [PATCH v5 4/5] memcg_stress_test.sh: use TST_TIMEOUT (replace LTP_TIMEOUT_MUL)

[Petr Vorel]

keeping the same timeout

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
index 5b19cc292..ad8605e16 100755
--- a/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
+++ b/testcases/kernel/controllers/memcg/stress/memcg_stress_test.sh
@@ -17,7 +17,7 @@ TST_NEEDS_CMDS="mount umount cat kill mkdir rmdir grep awk cut"
 
 # Each test case runs for 900 secs when everything fine
 # therefore the default 5 mins timeout is not enough.
-LTP_TIMEOUT_MUL=7
+TST_TIMEOUT=2100
 
 . cgroup_lib.sh
 
-- 
2.23.0

[LTP] [PATCH v5 5/5] net/if-mtu-change.sh: set TST_TIMEOUT

[Petr Vorel]

The default setup (100 * 5 seconds just for sleep) is longer than default timeout.
30 sec for each iteration should be more than enough as a default.

Fixes: fbea02ab5 ("lib/tst_test.sh: setup timeout per test run for the shell tests")

Reviewed-by: Li Wang <liwang@redhat.com>
Reviewed-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/network/stress/interface/if-mtu-change.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/testcases/network/stress/interface/if-mtu-change.sh b/testcases/network/stress/interface/if-mtu-change.sh
index 30c013214..b945fb6ce 100755
--- a/testcases/network/stress/interface/if-mtu-change.sh
+++ b/testcases/network/stress/interface/if-mtu-change.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 # SPDX-License-Identifier: GPL-2.0-or-later
-# Copyright (c) 2017-2018 Petr Vorel <pvorel@suse.cz>
+# Copyright (c) 2017-2019 Petr Vorel <pvorel@suse.cz>
 # Copyright (c) 2015-2017 Oracle and/or its affiliates. All Rights Reserved.
 # Copyright (c) International Business Machines  Corp., 2005
 # Author: Mitsuru Chinen <mitch@jp.ibm.com>
@@ -13,6 +13,8 @@ TST_CLEANUP="do_cleanup"
 # The interval of the mtu change [second]
 CHANGE_INTERVAL=${CHANGE_INTERVAL:-5}
 
+TST_TIMEOUT=$(((CHANGE_INTERVAL + 30) * MTU_CHANGE_TIMES))
+
 # The array of the value which MTU is changed into sequentially
 # 552 - net.ipv4.route.min_pmtu
 CHANGE_VALUES="784 1142 552 1500 552 1500 552 748 552 1142 1500"
-- 
2.23.0

[LTP] [PATCH v5 0/5] shell: Introduce TST_TIMEOUT variable

[Cyril Hrubis]

Hi!
> Changes v4->v5:
> * remove float related code (left from v3)
> * remove "tst_test_cmds cut" check from tst_test.sh (there is check for
> cut and other later => it should be probably removed as well, but as a
> separate patch) (Cyril)
> * remove unneeded IFS from test (Cyril)
> * mention ceiling LTP_TIMEOUT_MUL in doc/user-guide.txt
> 
> Here is the diff:
> diff --git doc/user-guide.txt doc/user-guide.txt
> index 8913c3221..7f6334ec2 100644
> --- doc/user-guide.txt
> +++ doc/user-guide.txt
> @@ -15,7 +15,7 @@ For running LTP network tests see `testcases/network/README.md`.
>                            'n' or '0': never colorize.
>  | 'LTP_TIMEOUT_MUL'     | Multiply timeout, must be number >= 1 (> 1 is useful for
>                            slow machines to avoid unexpected timeout).
> -                          Variable is also used in shell tests.
> +                          Variable is also used in shell tests, but here ceiled to int.
                                                                        ^
									Drop
									the
									"here"

Other than that it's fine, you can add my Reviewed-by:

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Clemens Famulla-Conrad]

On Fri, 2019-10-11 at 11:54 +0200, Petr Vorel wrote:
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  lib/newlib_tests/shell/test_timeout.sh | 36
> ++++++++++++++++++++++++++
>  lib/newlib_tests/shell/timeout01.sh    | 13 ++++++++++
>  lib/newlib_tests/shell/timeout02.sh    | 13 ++++++++++
>  3 files changed, 62 insertions(+)
>  create mode 100755 lib/newlib_tests/shell/test_timeout.sh
>  create mode 100755 lib/newlib_tests/shell/timeout01.sh
>  create mode 100755 lib/newlib_tests/shell/timeout02.sh
> 
> diff --git a/lib/newlib_tests/shell/test_timeout.sh
> b/lib/newlib_tests/shell/test_timeout.sh
> new file mode 100755
> index 000000000..2cbc66412
> --- /dev/null
> +++ b/lib/newlib_tests/shell/test_timeout.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +PATH="$(dirname $0)/../../../testcases/lib/:$PATH"
> +
> +DATA="
> +timeout01.sh||0

We only check if the lib doesn't produce any error, but we do not 
check if timeout is really unlimited. But I think we can do so when 
the shell-test-framework will be introduced and we can check for 
"TINFO: Timeout per run is disabled" output.

> +timeout02.sh||0
> +timeout02.sh|foo|32
> +timeout02.sh|2|0
> +timeout02.sh|1.1|0
> +timeout02.sh|-10|32

I think it is worth to add these tests as well:

timeout01.sh|2|0
timeout02.sh|-1.1|32
timeout02.sh|-10.1|32
timeout02.sh|-0.1|0

> +"
> +
> +echo "Testing timeout in shell API"
> +echo
> +
> +failed=0
> +for i in $DATA; do
> +	file=$(echo $i | cut -d'|' -f1)
> +	timeout=$(echo $i | cut -d'|' -f2)
> +	exp_exit=$(echo $i | cut -d'|' -f3)
> +
> +	echo "=== $test (LTP_TIMEOUT_MUL='$timeout') ==="
> +	LTP_TIMEOUT_MUL=$timeout ./$file
> +	ret=$?
> +	if [ $ret -ne $exp_exit ]; then
> +		echo "FAILED (exit code: $ret, expected $exp_exit)"
> +		failed=$((failed+1))
> +	else
> +		echo "PASSED"
> +	fi
> +	echo
> +done
> +
> +echo "Failed tests: $failed"
> +exit $failed
> diff --git a/lib/newlib_tests/shell/timeout01.sh
> b/lib/newlib_tests/shell/timeout01.sh
> new file mode 100755
> index 000000000..ab7428a2d
> --- /dev/null
> +++ b/lib/newlib_tests/shell/timeout01.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +TST_TESTFUNC=do_test
> +
> +TST_TIMEOUT=-1
> +. tst_test.sh
> +
> +do_test()
> +{
> +	tst_res TPASS "timeout $TST_TIMEOUT set"
> +}
> +
> +tst_run
> diff --git a/lib/newlib_tests/shell/timeout02.sh
> b/lib/newlib_tests/shell/timeout02.sh
> new file mode 100755
> index 000000000..73af09125
> --- /dev/null
> +++ b/lib/newlib_tests/shell/timeout02.sh
> @@ -0,0 +1,13 @@
> +#!/bin/sh
> +
> +TST_TESTFUNC=do_test
> +
> +TST_TIMEOUT=2
> +. tst_test.sh
> +
> +do_test()
> +{
> +	tst_res TPASS "timeout $TST_TIMEOUT set
> (LTP_TIMEOUT_MUL='$LTP_TIMEOUT_MUL')"
> +}
> +
> +tst_run

[LTP] [PATCH v5 0/5] shell: Introduce TST_TIMEOUT variable

[Clemens Famulla-Conrad]

On Fri, 2019-10-11 at 11:54 +0200, Petr Vorel wrote:
> Hi,
> 
> hopefully the latest version.
> 
> Changes v4->v5:
> * remove float related code (left from v3)
> * remove "tst_test_cmds cut" check from tst_test.sh (there is check
> for
> cut and other later => it should be probably removed as well, but as
> a
> separate patch) (Cyril)
> * remove unneeded IFS from test (Cyril)
> * mention ceiling LTP_TIMEOUT_MUL in doc/user-guide.txt
> 
> 
<snip>

For me this patchset looks good. I only add some nit comments on the
tests, can be taken or not.

Thanks
Clemens

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Petr Vorel]

Hi Clements,

thanks for your notes.

> > +DATA="
> > +timeout01.sh||0

> We only check if the lib doesn't produce any error, but we do not 
> check if timeout is really unlimited. But I think we can do so when 
> the shell-test-framework will be introduced and we can check for 
> "TINFO: Timeout per run is disabled" output.
Yes, I'd prefer to do more enhancements after shell-test-framework being
merged.

I plan to add also $(dirname $0) to PATH, so it can be run

-PATH="$(dirname $0)/../../../testcases/lib/:$PATH"
+PATH="$(dirname $0):$(dirname $0)/../../../testcases/lib/:$PATH"
...
-	LTP_TIMEOUT_MUL=$timeout ./$file
+	LTP_TIMEOUT_MUL=$timeout $file

And expect that shell-test-framework will handle this better.

> > +timeout02.sh||0
> > +timeout02.sh|foo|32
> > +timeout02.sh|2|0
> > +timeout02.sh|1.1|0
> > +timeout02.sh|-10|32

> I think it is worth to add these tests as well:

> timeout01.sh|2|0
> timeout02.sh|-1.1|32
> timeout02.sh|-10.1|32
> timeout02.sh|-0.1|0
OK, no problem to add them.

timeout02 1 TCONF: LTP_TIMEOUT_MUL must be number >= 1! (0)
BTW I wonder if TBROK shouldn't be used instead of TCONF.
Anybody strong opinion?

Kind regards,
Petr

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Cyril Hrubis]

Hi!
> timeout02 1 TCONF: LTP_TIMEOUT_MUL must be number >= 1! (0)
> BTW I wonder if TBROK shouldn't be used instead of TCONF.
> Anybody strong opinion?

If we fail to run a test because user passed wrong input data it has to
be TBROK because TCONF can end up unnoticed.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Petr Vorel]

Hi,

> > timeout02 1 TCONF: LTP_TIMEOUT_MUL must be number >= 1! (0)
> > BTW I wonder if TBROK shouldn't be used instead of TCONF.
> > Anybody strong opinion?

> If we fail to run a test because user passed wrong input data it has to
> be TBROK because TCONF can end up unnoticed.
+1.

I'd like to merge v5 with following diff:
Please let me know if anything else is problematic.

Kind regards,
Petr

diff --git lib/newlib_tests/shell/test_timeout.sh lib/newlib_tests/shell/test_timeout.sh
index 2cbc66412..948c7f02d 100755
--- lib/newlib_tests/shell/test_timeout.sh
+++ lib/newlib_tests/shell/test_timeout.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-PATH="$(dirname $0)/../../../testcases/lib/:$PATH"
+PATH="$(dirname $0):$(dirname $0)/../../../testcases/lib/:$PATH"
 
 DATA="
 timeout01.sh||0
@@ -9,6 +9,11 @@ timeout02.sh|foo|32
 timeout02.sh|2|0
 timeout02.sh|1.1|0
 timeout02.sh|-10|32
+
+timeout01.sh|2|0
+timeout02.sh|-1.1|32
+timeout02.sh|-10.1|32
+timeout02.sh|-0.1|0
 "
 
 echo "Testing timeout in shell API"
@@ -21,7 +26,7 @@ for i in $DATA; do
 	exp_exit=$(echo $i | cut -d'|' -f3)
 
 	echo "=== $test (LTP_TIMEOUT_MUL='$timeout') ==="
-	LTP_TIMEOUT_MUL=$timeout ./$file
+	LTP_TIMEOUT_MUL=$timeout $file
 	ret=$?
 	if [ $ret -ne $exp_exit ]; then
 		echo "FAILED (exit code: $ret, expected $exp_exit)"
diff --git testcases/lib/tst_test.sh testcases/lib/tst_test.sh
index 8713c1cdd..d8071cb10 100644
--- testcases/lib/tst_test.sh
+++ testcases/lib/tst_test.sh
@@ -389,14 +389,14 @@ _tst_setup_timer()
 
 	local err="LTP_TIMEOUT_MUL must be number >= 1!"
 
-	tst_is_num "$LTP_TIMEOUT_MUL" || tst_brk TCONF "$err ($LTP_TIMEOUT_MUL)"
+	tst_is_num "$LTP_TIMEOUT_MUL" || tst_brk TBROK "$err ($LTP_TIMEOUT_MUL)"
 
 	if ! tst_is_int "$LTP_TIMEOUT_MUL"; then
 		LTP_TIMEOUT_MUL=$(echo "$LTP_TIMEOUT_MUL" | cut -d. -f1)
 		LTP_TIMEOUT_MUL=$((LTP_TIMEOUT_MUL+1))
 		tst_res TINFO "ceiling LTP_TIMEOUT_MUL to $LTP_TIMEOUT_MUL"
 	fi
-	[ "$LTP_TIMEOUT_MUL" -ge 1 ] || tst_brk TCONF "$err ($LTP_TIMEOUT_MUL)"
+	[ "$LTP_TIMEOUT_MUL" -ge 1 ] || tst_brk TBROK "$err ($LTP_TIMEOUT_MUL)"
 
 	if ! tst_is_int "$TST_TIMEOUT" || [ "$TST_TIMEOUT" -lt 1 ]; then
 		tst_brk TBROK "TST_TIMEOUT must be int >= 1! ($TST_TIMEOUT)"

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Cyril Hrubis]

Hi!
Fine with me.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Li Wang]

On Fri, Oct 11, 2019 at 9:31 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi,
>
> > > timeout02 1 TCONF: LTP_TIMEOUT_MUL must be number >= 1! (0)
> > > BTW I wonder if TBROK shouldn't be used instead of TCONF.
> > > Anybody strong opinion?
>
> > If we fail to run a test because user passed wrong input data it has to
> > be TBROK because TCONF can end up unnoticed.
> +1.
>
> I'd like to merge v5 with following diff:
> Please let me know if anything else is problematic.
>

Thanks for your work Petr!
For patchset:
    Tested-by: Li Wang <liwang@redhat.com>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20191012/4a38de5d/attachment.htm>

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Clemens Famulla-Conrad]

Ack

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Petr Vorel]

Hi,

> Ack
Thanks a lot, merged.

Kind regards,
Petr

[LTP] [PATCH v5 3/5] shell: Add timeout shell API tests

[Petr Vorel]

Hi Li,

> Thanks for your work Petr!
> For patchset:
>     Tested-by: Li Wang <liwang@redhat.com>
I'm sorry, forget to add this tag.
Thanks for testing!

Kind regards,
Petr