diff mbox series

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

Message ID 20191011095442.10541-4-pvorel@suse.cz
State Accepted
Delegated to: Petr Vorel
Headers show
Series [v5,1/5] shell: Add tst_is_num() | expand

Commit Message

Petr Vorel Oct. 11, 2019, 9:54 a.m. UTC 
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

Comments

Clemens Famulla-Conrad Oct. 11, 2019, 12:36 p.m. UTC | #1 
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
Petr Vorel Oct. 11, 2019, 12:54 p.m. UTC | #2 
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
Cyril Hrubis Oct. 11, 2019, 12:56 p.m. UTC | #3 
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.
Petr Vorel Oct. 11, 2019, 1:31 p.m. UTC | #4 
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)"
Cyril Hrubis Oct. 11, 2019, 2:02 p.m. UTC | #5 
Hi!
Fine with me.
Li Wang Oct. 12, 2019, 9:55 a.m. UTC | #6 
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>
Clemens Famulla-Conrad Oct. 14, 2019, 8:08 a.m. UTC | #7 
Ack
Petr Vorel Oct. 14, 2019, 8:46 a.m. UTC | #8 
Hi,

> Ack
Thanks a lot, merged.

Kind regards,
Petr
Petr Vorel Oct. 14, 2019, 8:47 a.m. UTC | #9 
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
diff mbox series

Patch

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