[LTP] [PATCH v2 0/3] LTP_TIMEOUT for shell API

[LTP] [PATCH v2 0/3] LTP_TIMEOUT for shell API

[Petr Vorel]

Hi,

changes v1->v2:
addressed issues reported by Clemens and Cristian (thanks!)
* s/LTP_TIMEOUT/TST_TIMEOUT in whole patchset (assume for test variable
is better to stick with TST_ prefix)
* fix int/float $LTP_TIMEOUT_MUL detection algorithm (added missing parameter "$LTP_TIMEOUT_MUL"

+ other fixes:
* add "TIMEOUT" to filter in tst_run() (to filter out TST_TIMEOUT)
* round $sec off to make it being int, so that for $h $m $s calculations
it's possible to use $(( )).

Questions:
* I don't know how to detect TST_TIMEOUT settings made by user. That's
the difference from C, where user cannot overwrite tst_test->test.
Am I missing something or it's not possible to detect whether variable
was set in code or by user?
Maybe that was the reason TST_TIMEOUT wasn't set, but rather fixed.

* Code allowing $LTP_TIMEOUT_MUL being also float making code a bit
complex. If you don't like it, I suggest to change $LTP_TIMEOUT_MUL
being for both C and shell integer, but I'd prefer the possibility to be float.
(it's might be handy being able to set timeout 1.33x which is far less
than 2x for int).

Kind regards,
Petr

Petr Vorel (3):
  shell: Add tst_is_num()
  shell: Introduce TST_TIMEOUT variable
  net/if-mtu-change.sh: set TST_TIMEOUT

 doc/test-writing-guidelines.txt               | 89 ++++++++++++-------
 .../memcg/stress/memcg_stress_test.sh         |  2 +-
 testcases/lib/tst_test.sh                     | 43 ++++++++-
 .../network/stress/interface/if-mtu-change.sh |  4 +-
 4 files changed, 101 insertions(+), 37 deletions(-)

-- 
2.22.1

[LTP] [PATCH v2 1/3] shell: Add tst_is_num()

[Petr Vorel]

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

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 a735b43bb..2d118578f 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -2259,6 +2259,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.22.1

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[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 that adds awk as a dependency.

+ replace LTP_TIMEOUT_MUL in memcg_stress_test.sh with TST_TIMEOUT.

+ document both variables.

Suggested-by: Clemens Famulla-Conrad <cfamullaconrad@suse.de>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/test-writing-guidelines.txt               | 83 ++++++++++++-------
 .../memcg/stress/memcg_stress_test.sh         |  2 +-
 testcases/lib/tst_test.sh                     | 36 +++++++-
 3 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 2d118578f..47132e9a7 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -488,7 +488,18 @@ before calling 'fork()' or 'clone()'. Note that the 'SAFE_FORK()' calls this
 function automatically. See 3.4 FILE buffers and fork() for explanation why is
 this needed.
 
-2.2.3 Test temporary directory
+2.2.3 Library environment variables for C
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+[options="header"]
+|=============================================================================
+| Variable name      | Action done
+| '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.
+|=============================================================================
+
+2.2.4 Test temporary directory
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 If '.needs_tmpdir' is set to '1' in the 'struct tst_test' unique test
@@ -500,7 +511,7 @@ IMPORTANT: Close all file descriptors (that point to files in test temporary
 	   or in the test 'cleanup()' otherwise the test may break temporary
 	   directory removal on NFS (look for "NFS silly rename").
 
-2.2.4 Safe macros
+2.2.5 Safe macros
 ^^^^^^^^^^^^^^^^^
 
 Safe macros aim to simplify error checking in test preparation. Instead of
@@ -539,7 +550,7 @@ example, do:
 See 'include/tst_safe_macros.h', 'include/tst_safe_stdio.h' and
 'include/tst_safe_file_ops.h' and 'include/tst_safe_net.h' for a complete list.
 
-2.2.5 Test specific command line options
+2.2.6 Test specific command line options
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -617,7 +628,7 @@ static struct tst_test test = {
 -------------------------------------------------------------------------------
 
 
-2.2.6 Runtime kernel version detection
+2.2.7 Runtime kernel version detection
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Testcases for newly added kernel functionality require kernel newer than a
@@ -655,7 +666,7 @@ test may be relevant even if the kernel version does not suggests so. See
 WARNING: The shell 'tst_kvercmp' maps the result into unsigned integer - the
          process exit value.
 
-2.2.7 Fork()-ing
+2.2.8 Fork()-ing
 ^^^^^^^^^^^^^^^^
 
 Be wary that if the test forks and there were messages printed by the
@@ -671,7 +682,7 @@ To avoid that you should use 'SAFE_FORK()'.
 IMPORTANT: You have to set the '.forks_child' flag in the test structure
            if your testcase forks.
 
-2.2.8 Doing the test in the child process
+2.2.9 Doing the test in the child process
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Results reported by 'tst_res()' are propagated to the parent test process via
@@ -747,7 +758,7 @@ library prepares for it and has to make sure the 'LTP_IPC_PATH' environment
 variable is passed down, then the very fist thing the program has to call in
 'main()' is 'tst_reinit()' that sets up the IPC.
 
-2.2.9 Fork() and Parent-child synchronization
+2.2.10 Fork() and Parent-child synchronization
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 As LTP tests are written for Linux, most of the tests involve fork()-ing and
@@ -826,7 +837,7 @@ The 'TST_PROCESS_STATE_WAIT()' waits until process 'pid' is in requested
 It's mostly used with state 'S' which means that process is sleeping in kernel
 for example in 'pause()' or any other blocking syscall.
 
-2.2.10 Signals and signal handlers
+2.2.11 Signals and signal handlers
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 If you need to use signal handlers, keep the code short and simple. Don't
@@ -867,14 +878,14 @@ they subsequently modify RLIMIT_CORE.
 Note that LTP library will reap any processes that test didn't reap itself,
 and report any non-zero exit code as failure.
 
-2.2.11 Kernel Modules
+2.2.12 Kernel Modules
 ^^^^^^^^^^^^^^^^^^^^^
 
 There are certain cases where the test needs a kernel part and userspace part,
 happily, LTP can build a kernel module and then insert it to the kernel on test
 start for you. See 'testcases/kernel/device-drivers/block' for details.
 
-2.2.12 Useful macros
+2.2.13 Useful macros
 ^^^^^^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -892,7 +903,7 @@ LTP_ALIGN(x, a)
 
 Aligns the x to be next multiple of a. The a must be power of 2.
 
-2.2.13 Filesystem type detection
+2.2.14 Filesystem type detection
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests are known to fail on certain filesystems (you cannot swap on TMPFS,
@@ -927,7 +938,7 @@ below:
 	}
 -------------------------------------------------------------------------------
 
-2.2.14 Thread-safety in the LTP library
+2.2.15 Thread-safety in the LTP library
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 It is safe to use library 'tst_res()' function in multi-threaded tests.
@@ -979,7 +990,7 @@ static void cleanup(void)
 -------------------------------------------------------------------------------
 
 
-2.2.15 Testing with a block device
+2.2.16 Testing with a block device
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests needs a block device (inotify tests, syscall 'EROFS' failures,
@@ -1071,7 +1082,7 @@ unsigned long tst_dev_bytes_written(const char *dev);
 This function reads test block device stat file (/sys/block/<device>/stat) and
 returns the bytes written since the last invocation of this function.
 
-2.2.16 Formatting a device with a filesystem
+2.2.17 Formatting a device with a filesystem
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -1098,7 +1109,7 @@ The extra options 'extra_opts' should either be 'NULL' if there are none, or a
 will be passed after device name. e.g: +mkfs -t ext4 -b 1024 /dev/sda1 102400+
 in this case.
 
-2.2.17 Verifying a filesystem's free space
+2.2.18 Verifying a filesystem's free space
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests have size requirements for the filesystem's free space. If these
@@ -1123,7 +1134,7 @@ The required free space is calculated by 'size * mult', e.g.
 filesystem, which '"/tmp/testfile"' is in, has 64MB free space at least, and 0
 if not.
 
-2.2.18 Files, directories and fs limits
+2.2.19 Files, directories and fs limits
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests need to know the maximum count of links to a regular file or
@@ -1198,7 +1209,7 @@ int tst_fill_file(const char *path, char pattern, size_t bs, size_t bcount);
 
 Creates/overwrites a file with specified pattern using file path.
 
-2.2.19 Getting an unused PID number
+2.2.20 Getting an unused PID number
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests require a 'PID', which is not used by the OS (does not belong to
@@ -1224,7 +1235,7 @@ int tst_get_free_pids(void);
 Returns number of unused pids in the system. Note that this number may be
 different once the call returns and should be used only for rough estimates.
 
-2.2.20 Running executables
+2.2.21 Running executables
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -1266,7 +1277,7 @@ const char *const cmd[] = { "ls", "-l", NULL };
 ...
 -------------------------------------------------------------------------------
 
-2.2.21 Measuring elapsed time and helper functions
+2.2.22 Measuring elapsed time and helper functions
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -1380,7 +1391,7 @@ between two times.
 
 NOTE: All conversions to ms and us rounds the value.
 
-2.2.22 Datafiles
+2.2.23 Datafiles
 ^^^^^^^^^^^^^^^^
 
 [source,c]
@@ -1418,7 +1429,7 @@ was installed.
 The file(s) are copied to the newly created test temporary directory which is
 set as the test working directory when the 'test()' functions is executed.
 
-2.2.23 Code path tracing
+2.2.24 Code path tracing
 ^^^^^^^^^^^^^^^^^^^^^^^^
 
 'tst_res' is a macro, so on when you define a function in one file:
@@ -1465,7 +1476,7 @@ common.h:9: FAIL: check failed
 test.c:8: INFO: do_action(arg) failed
 -------------------------------------------------------------------------------
 
-2.2.24 Tainted kernels
+2.2.25 Tainted kernels
 ^^^^^^^^^^^^^^^^^^^^^^
 
 If you need to detect, if a testcase triggers a kernel warning, bug or oops,
@@ -1507,13 +1518,13 @@ For reference to tainted kernels, see kernel documentation:
 Documentation/admin-guide/tainted-kernels.rst or
 https://www.kernel.org/doc/html/latest/admin-guide/tainted-kernels.html
 
-2.2.25 Checksums
+2.2.26 Checksums
 ^^^^^^^^^^^^^^^^
 
 CRC32c checksum generation is supported by LTP. In order to use it, the
 test should include "tst_checksum.h" header, then can call tst_crc32c().
 
-2.2.26 Checking kernel for the driver support
+2.2.27 Checking kernel for the driver support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests may need specific kernel drivers, either compiled in, or built
@@ -1524,7 +1535,7 @@ first missing driver.
 Since it relies on modprobe command, the check will be skipped if the command
 itself is not available on the system.
 
-2.2.27 Saving & restoring /proc|sys values
+2.2.28 Saving & restoring /proc|sys values
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 LTP library can be instructed to save and restore value of specified
@@ -1563,7 +1574,7 @@ static struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
-2.2.28 Parsing kernel .config
+2.2.29 Parsing kernel .config
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Generally testcases should attempt to autodetect as much kernel features as
@@ -1598,7 +1609,7 @@ static struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
-2.2.29 Changing the Wall Clock Time during test execution
+2.2.30 Changing the Wall Clock Time during test execution
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 There are some tests that, for different reasons, might need to change the
@@ -1634,7 +1645,7 @@ struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
-2.2.30 Testing similar syscalls in one test
+2.2.31 Testing similar syscalls in one test
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 In some cases kernel has several very similar syscalls that do either the same
@@ -1699,7 +1710,7 @@ struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
-2.2.31 Guarded buffers
+2.2.32 Guarded buffers
 ^^^^^^^^^^^^^^^^^^^^^^
 
 The test library supports guarded buffers, which are buffers allocated so
@@ -1769,7 +1780,7 @@ setting up the size or struct iovec, which is allocated recursively including
 the individual buffers as described by an '-1' terminated array of buffer
 sizes.
 
-2.2.32 Adding and removing capabilities
+2.2.33 Adding and removing capabilities
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Some tests may require the presence or absence of particular
@@ -2030,8 +2041,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'.
@@ -2047,6 +2058,14 @@ 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 float
+                       >= 1 for C, int >=1 for shell. Default value is 300 sec,
+                       timeout can be disabled by setting it to -1.
+                       Variable should be set in tests, not by user.
+| 'LTP_TIMEOUT_MUL'  | Multiply timeout, must be number >= 1 (> 1 is useful for
+                       slow machines to avoid unexpected timeout).
+                       Variable is also used in C tests,
+                       it should be set by user, not in tests.
 |=============================================================================
 
 NOTE: Network tests (see testcases/network/README.md) use additional variables
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
 
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index ca63745fd..b206fc0bb 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -379,9 +379,41 @@ _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 is_float
+	if tst_is_num "$LTP_TIMEOUT_MUL"; then
+		if tst_is_int "$LTP_TIMEOUT_MUL"; then
+			[ "$LTP_TIMEOUT_MUL" -ge 1 ] || err=1
+		else
+			tst_test_cmds awk
+			echo | awk '{if ('"$LTP_TIMEOUT_MUL"' < 1) {exit 1}}' || err=1
+			is_float=1
+		fi
+	else
+		err=1
+	fi
+	if [ "$err" ]; then
+		tst_brk TCONF "LTP_TIMEOUT_MUL must be number >= 1! ($LTP_TIMEOUT_MUL)"
+	fi
+
+	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
+	if [ "$is_float" ]; then
+		sec=`echo |awk '{printf("%d\n", '$TST_TIMEOUT' * '$LTP_TIMEOUT_MUL')}'`
+	else
+		sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
+	fi
+
 	local h=$((sec / 3600))
 	local m=$((sec / 60 % 60))
 	local s=$((sec % 60))
@@ -418,7 +450,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.22.1

[LTP] [PATCH v2 3/3] 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")

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.22.1

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Li Wang]

Hi Petr,

Thanks for your working.

On Fri, Sep 13, 2019 at 8:58 PM Petr Vorel <pvorel@suse.cz> wrote:

>
> <snip>

>
> --- 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
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index ca63745fd..b206fc0bb 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -379,9 +379,41 @@ _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 is_float
> +       if tst_is_num "$LTP_TIMEOUT_MUL"; then
> +               if tst_is_int "$LTP_TIMEOUT_MUL"; then
> +                       [ "$LTP_TIMEOUT_MUL" -ge 1 ] || err=1
> +               else
> +                       tst_test_cmds awk
> +                       echo | awk '{if ('"$LTP_TIMEOUT_MUL"' < 1) {exit
> 1}}' || err=1
> +                       is_float=1
> +               fi
> +       else
> +               err=1
> +       fi
>

I'm OK to allow $LTP_TIMEOUT_MUL being float. But here I don't see what's
enough reason to add the is_float variable. Because we could use the float
expression for both types comparing, isn't it?


> +       if [ "$err" ]; then
> +               tst_brk TCONF "LTP_TIMEOUT_MUL must be number >= 1!
> ($LTP_TIMEOUT_MUL)"
> +       fi
> +
> +       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
> +       if [ "$is_float" ]; then
> +               sec=`echo |awk '{printf("%d\n", '$TST_TIMEOUT' *
> '$LTP_TIMEOUT_MUL')}'`
> +       else
> +               sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
> +       fi
>

Here as well, why we need to distinguish the float and int, is the float
expression does not work for integer?

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

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Clemens Famulla-Conrad]

Hi Petr,
only some small comments below.

On Fri, 2019-09-13 at 14:58 +0200, Petr Vorel wrote:
> <snip>
> -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'.
> @@ -2047,6 +2058,14 @@ 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 float

      ^
I think TST_TIMEOUT isn't evaluated in c at all. There we have `(struct
tst_test*)->timeout` which is `int`,

> +                       >= 1 for C, int >=1 for shell. Default value
> is 300 sec,
> +                       timeout can be disabled by setting it to -1.
> +                       Variable should be set in tests, not by user.
> +| 'LTP_TIMEOUT_MUL'  | Multiply timeout, must be number >= 1 (> 1 is
> useful for
> +                       slow machines to avoid unexpected timeout).
> +                       Variable is also used in C tests,
> +                       it should be set by user, not in tests.
>  |===================================================================
> ==========
>  
>  NOTE: Network tests (see testcases/network/README.md) use additional
> variables
> 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
>  
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index ca63745fd..b206fc0bb 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -379,9 +379,41 @@ _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 is_float
> +	if tst_is_num "$LTP_TIMEOUT_MUL"; then
> +		if tst_is_int "$LTP_TIMEOUT_MUL"; then
> +			[ "$LTP_TIMEOUT_MUL" -ge 1 ] || err=1
> +		else
> +			tst_test_cmds awk
> +			echo | awk '{if ('"$LTP_TIMEOUT_MUL"' < 1)
> {exit 1}}' || err=1
> +			is_float=1
> +		fi
> +	else
> +		err=1
> +	fi
> +	if [ "$err" ]; then
> +		tst_brk TCONF "LTP_TIMEOUT_MUL must be number >= 1!
> ($LTP_TIMEOUT_MUL)"
> +	fi
> +
> +	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
> +	if [ "$is_float" ]; then
> +		sec=`echo |awk '{printf("%d\n", '$TST_TIMEOUT' * 

                          ^
                          nit, space after |

> '$LTP_TIMEOUT_MUL')}'`

                   ^
                   + 0.5
In C implementation we round up. Maybe we should do the same in shell.

> +	else
> +		sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
> +	fi
> +
>  	local h=$((sec / 3600))
>  	local m=$((sec / 60 % 60))
>  	local s=$((sec % 60))
> @@ -418,7 +450,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

[LTP] [PATCH v2 0/3] LTP_TIMEOUT for shell API

[Clemens Famulla-Conrad]

On Fri, 2019-09-13 at 14:58 +0200, Petr Vorel wrote:
<snip>
> Questions:
> * I don't know how to detect TST_TIMEOUT settings made by user.
> That's
> the difference from C, where user cannot overwrite tst_test->test.
> Am I missing something or it's not possible to detect whether
> variable
> was set in code or by user?
> Maybe that was the reason TST_TIMEOUT wasn't set, but rather fixed.

Maybe we could initialize all variables in tst_test.sh. So we overwrite
 values given by user/environment. But I in general I think, if someone
play with TST_* variables, it's up to him what happen.
And you already added this WARNING!

> * Code allowing $LTP_TIMEOUT_MUL being also float making code a bit
> complex. If you don't like it, I suggest to change $LTP_TIMEOUT_MUL
> being for both C and shell integer, but I'd prefer the possibility to
> be float.
> (it's might be handy being able to set timeout 1.33x which is far
> less
> than 2x for int).

I also don't see the need of such fine granular multiplier.

[LTP] [PATCH v2 0/3] LTP_TIMEOUT for shell API

[Petr Vorel]

Hi,

> <snip>
> > Questions:
> > * I don't know how to detect TST_TIMEOUT settings made by user.
> > That's
> > the difference from C, where user cannot overwrite tst_test->test.
> > Am I missing something or it's not possible to detect whether
> > variable
> > was set in code or by user?
> > Maybe that was the reason TST_TIMEOUT wasn't set, but rather fixed.

> Maybe we could initialize all variables in tst_test.sh. So we overwrite
>  values given by user/environment.
But that's also overwrite setting in the test (in this patchset it's added in
memcg_stress_test.sh). I.e. library don't know whether variable is set by user
or a test.

> But I in general I think, if someone
> play with TST_* variables, it's up to him what happen.
> And you already added this WARNING!
Yep.

> > * Code allowing $LTP_TIMEOUT_MUL being also float making code a bit
> > complex. If you don't like it, I suggest to change $LTP_TIMEOUT_MUL
> > being for both C and shell integer, but I'd prefer the possibility to
> > be float.
> > (it's might be handy being able to set timeout 1.33x which is far
> > less
> > than 2x for int).

> I also don't see the need of such fine granular multiplier.
Then it should be unified, i.e. force to use int instead of float also in C API.

Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

> Hi Petr,

> Thanks for your working.
Thanks for a review!

...
> > +++ b/testcases/lib/tst_test.sh
> > @@ -379,9 +379,41 @@ _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 is_float
> > +       if tst_is_num "$LTP_TIMEOUT_MUL"; then
> > +               if tst_is_int "$LTP_TIMEOUT_MUL"; then
> > +                       [ "$LTP_TIMEOUT_MUL" -ge 1 ] || err=1
> > +               else
> > +                       tst_test_cmds awk
This is the reason: awk dependency.

> > +                       echo | awk '{if ('"$LTP_TIMEOUT_MUL"' < 1) {exit
> > 1}}' || err=1
> > +                       is_float=1
> > +               fi
> > +       else
> > +               err=1
> > +       fi


> I'm OK to allow $LTP_TIMEOUT_MUL being float. But here I don't see what's
> enough reason to add the is_float variable. Because we could use the float
> expression for both types comparing, isn't it?


> > +       if [ "$err" ]; then
> > +               tst_brk TCONF "LTP_TIMEOUT_MUL must be number >= 1!
> > ($LTP_TIMEOUT_MUL)"
> > +       fi
> > +
> > +       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
> > +       if [ "$is_float" ]; then
> > +               sec=`echo |awk '{printf("%d\n", '$TST_TIMEOUT' *
> > '$LTP_TIMEOUT_MUL')}'`
> > +       else
> > +               sec=$((TST_TIMEOUT * LTP_TIMEOUT_MUL))
> > +       fi


> Here as well, why we need to distinguish the float and int, is the float
> expression does not work for integer?
Because it brings awk dependency on whole library, which I'm not sure if we
want (awk is a must on linux distros, it's in busybox; it's missing on android
toolsbox, but android project concentrates on syscalls, probably nobody uses
shell tests on android). Also I'm not sure about containers, JeOS etc. (it
sometimes miss a basic dependency).
If awk dependency is ok, it'd simplify test a bit.

Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Li Wang]

> > Here as well, why we need to distinguish the float and int, is the float
> > expression does not work for integer?
> Because it brings awk dependency on whole library, which I'm not sure if we
> want (awk is a must on linux distros, it's in busybox; it's missing on
> android
> toolsbox, but android project concentrates on syscalls, probably nobody
> uses
> shell tests on android). Also I'm not sure about containers, JeOS etc. (it
> sometimes miss a basic dependency).
> If awk dependency is ok, it'd simplify test a bit.


But it doesn't solve the dependency issue because, if the awk is missing,
it will still tst_brk on float $LTP_TIMEOUT_MUL at:
    tst_test_cmds awk
and never goes to the integer expression branch.

So, maybe that could be as a reason to disable float support for
$LTP_TIMEOUT_MUL?

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

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

Hi Li,

> > > Here as well, why we need to distinguish the float and int, is the float
> > > expression does not work for integer?
> > Because it brings awk dependency on whole library, which I'm not sure if we
> > want (awk is a must on linux distros, it's in busybox; it's missing on
> > android
> > toolsbox, but android project concentrates on syscalls, probably nobody
> > uses
> > shell tests on android). Also I'm not sure about containers, JeOS etc. (it
> > sometimes miss a basic dependency).
> > If awk dependency is ok, it'd simplify test a bit.


> But it doesn't solve the dependency issue because, if the awk is missing,
> it will still tst_brk on float $LTP_TIMEOUT_MUL at:
>     tst_test_cmds awk
No, int was handled previously in:
      if tst_is_int "$LTP_TIMEOUT_MUL"; then
> and never goes to the integer expression branch.

> So, maybe that could be as a reason to disable float support for
> $LTP_TIMEOUT_MUL?

Maybe I'm missing something, therefore explaining the intent of the code.
(int was handled in if tst_is_int "$LTP_TIMEOUT_MUL"; then):

+	local err is_float
+	if tst_is_num "$LTP_TIMEOUT_MUL"; then
		# it's int or float
+		if tst_is_int "$LTP_TIMEOUT_MUL"; then
			# it's int only
+			[ "$LTP_TIMEOUT_MUL" -ge 1 ] || err=1
+		else
            # it's float only
+			tst_test_cmds awk
+			echo | awk '{if ('"$LTP_TIMEOUT_MUL"' < 1) {exit 1}}' || err=1
+			is_float=1
+		fi
+	else
        # NaN
+		err=1
+	fi
+	if [ "$err" ]; then
+		tst_brk TCONF "LTP_TIMEOUT_MUL must be number >= 1! ($LTP_TIMEOUT_MUL)"
+	fi

Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Li Wang]

Hi Petr,

On Wed, Sep 18, 2019 at 4:24 PM Petr Vorel <pvorel@suse.cz> wrote:

> ...
>
> > So, maybe that could be as a reason to disable float support for
> > $LTP_TIMEOUT_MUL?
>
> Maybe I'm missing something, therefore explaining the intent of the code.
> (int was handled in if tst_is_int "$LTP_TIMEOUT_MUL"; then):
>
I'm not blaming the int/float judgment, there is no problem. My concern is:

If the $LTP_TIMEOUT_MUL is float and awk command is missing, how things
will be going?

It will break at:
    tst_test_cmds awk
right?

Given that break on float number handling, why not declare only support
integer for $LTP_TIMEOUT_MUL?

I hope I explained clearly this time, haha ;-)

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

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

Hi Clements,

> Hi Petr,
> only some small comments below.

> On Fri, 2019-09-13 at 14:58 +0200, Petr Vorel wrote:
> > <snip>
> > -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'.
> > @@ -2047,6 +2058,14 @@ 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 float

>       ^
> I think TST_TIMEOUT isn't evaluated in c at all. There we have `(struct
> tst_test*)->timeout` which is `int`,
> tst_test*)->timeout (BTW it's *unsigned* int).
Correct, thanks!
This is a proposed description, which I'll post to v3.
(using tst_test.timeout to be short enough, is that ok?):
| '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.
| 'LTP_TIMEOUT_MUL'  | Multiply timeout, must be number >= 1 (> 1 is useful for
                       slow machines to avoid unexpected timeout).
                       Variable is also used in C tests.
                       It's meant to be set by user, not in tests.
...
> > +	local sec
> > +	if [ "$is_float" ]; then
> > +		sec=`echo |awk '{printf("%d\n", '$TST_TIMEOUT' * 

>                           ^
>                           nit, space after |
Sure :).

> > '$LTP_TIMEOUT_MUL')}'`

>                    ^
>                    + 0.5
> In C implementation we round up. Maybe we should do the same in shell.
Correct, thanks!


Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

Hi Li,

> On Wed, Sep 18, 2019 at 4:24 PM Petr Vorel <pvorel@suse.cz> wrote:

> > ...

> > > So, maybe that could be as a reason to disable float support for
> > > $LTP_TIMEOUT_MUL?

> > Maybe I'm missing something, therefore explaining the intent of the code.
> > (int was handled in if tst_is_int "$LTP_TIMEOUT_MUL"; then):

> I'm not blaming the int/float judgment, there is no problem. My concern is:

> If the $LTP_TIMEOUT_MUL is float and awk command is missing, how things
> will be going?

> It will break at:
>     tst_test_cmds awk
> right?
Yes. The concern is described in the commit message (but it should also be in
wiki page I guess): you want float => you need awk. You don't have float, just
set it as int (which will affect also C).
Does it make sense? Is it useful?

> Given that break on float number handling, why not declare only support
> integer for $LTP_TIMEOUT_MUL?
Sure, we can do it. But I propose to do it for C as well otherwise setup valid
for C will be breaking shell. But that's a backward incompatibility change.
That's why I suggested IMHO the least intrusive change (but maybe I'm wrong).
Anyone else having strong opinion?

> I hope I explained clearly this time, haha ;-)
Sure, sorry to be slow :).

Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Li Wang]

Hi Petr,

On Wed, Sep 18, 2019 at 5:50 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > On Wed, Sep 18, 2019 at 4:24 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > ...
>
> > > > So, maybe that could be as a reason to disable float support for
> > > > $LTP_TIMEOUT_MUL?
>
> > > Maybe I'm missing something, therefore explaining the intent of the
> code.
> > > (int was handled in if tst_is_int "$LTP_TIMEOUT_MUL"; then):
>
> > I'm not blaming the int/float judgment, there is no problem. My concern
> is:
>
> > If the $LTP_TIMEOUT_MUL is float and awk command is missing, how things
> > will be going?
>
> > It will break at:
> >     tst_test_cmds awk
> > right?
> Yes. The concern is described in the commit message (but it should also be
> in
> wiki page I guess): you want float => you need awk. You don't have float,
> just
> set it as int (which will affect also C).
> Does it make sense? Is it useful?
>

It's working, but not elegant. Not sure how many people will go through the
documents for such tiny issues.

Is there any possibility to handle float by default, and if no awk
supporting, just round up the float to integer and use it do testing
automatically?


>
> > Given that break on float number handling, why not declare only support
> > integer for $LTP_TIMEOUT_MUL?
> Sure, we can do it. But I propose to do it for C as well otherwise setup
> valid
> for C will be breaking shell. But that's a backward incompatibility change.
> That's why I suggested IMHO the least intrusive change (but maybe I'm
> wrong).
> Anyone else having strong opinion?
>

No more concerns.


>
> > I hope I explained clearly this time, haha ;-)
> Sure, sorry to be slow :).
>

Never mind, I need to enhance my expression too.


>
> Kind regards,
> Petr
>

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

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Petr Vorel]

Hi Li,

> > > If the $LTP_TIMEOUT_MUL is float and awk command is missing, how things
> > > will be going?

> > > It will break at:
> > >     tst_test_cmds awk
> > > right?
> > Yes. The concern is described in the commit message (but it should also be
> > in
> > wiki page I guess): you want float => you need awk. You don't have float,
> > just
> > set it as int (which will affect also C).
> > Does it make sense? Is it useful?


> It's working, but not elegant. Not sure how many people will go through the
> documents for such tiny issues.

> Is there any possibility to handle float by default, and if no awk
> supporting, just round up the float to integer and use it do testing
> automatically?
Yep, that's better idea, thanks!
I'll print some info in case of forcing to round to int in v3.

Kind regards,
Petr

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Clemens Famulla-Conrad]

Hi Petr,

On Wed, 2019-09-18 at 11:40 +0200, Petr Vorel wrote:
> Hi Clements,
> 
> > Hi Petr,
> > only some small comments below.
> > On Fri, 2019-09-13 at 14:58 +0200, Petr Vorel wrote:
> > > <snip>
> > > -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'.
> > > @@ -2047,6 +2058,14 @@ 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 float
> >       ^
> > I think TST_TIMEOUT isn't evaluated in c at all. There we have
> > `(struct
> > tst_test*)->timeout` which is `int`,
> > tst_test*)->timeout (BTW it's *unsigned* int).
> 
> Correct, thanks!
> This is a proposed description, which I'll post to v3.
> (using tst_test.timeout to be short enough, is that ok?):
> > '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.
> > 'LTP_TIMEOUT_MUL'  | Multiply timeout, must be number >= 1 (> 1 is
> > useful for
> 
>                        slow machines to avoid unexpected timeout).
>                        Variable is also used in C tests.
>                        It's meant to be set by user, not in tests.

ACK

[LTP] [PATCH v2 2/3] shell: Introduce TST_TIMEOUT variable

[Clemens Famulla-Conrad]

On Wed, 2019-09-18 at 15:53 +0200, Petr Vorel wrote:
> Hi Li,
> 
> > > > If the $LTP_TIMEOUT_MUL is float and awk command is missing,
> > > > how things
> > > > will be going?
> > > > It will break at:
> > > >     tst_test_cmds awk
> > > > right?
> > > 
> > > Yes. The concern is described in the commit message (but it
> > > should also be
> > > in
> > > wiki page I guess): you want float => you need awk. You don't
> > > have float,
> > > just
> > > set it as int (which will affect also C).
> > > Does it make sense? Is it useful?
> 
> 
> > It's working, but not elegant. Not sure how many people will go
> > through the
> > documents for such tiny issues.
> > Is there any possibility to handle float by default, and if no awk
> > supporting, just round up the float to integer and use it do
> > testing
> > automatically?
> 
> Yep, that's better idea, thanks!
> I'll print some info in case of forcing to round to int in v3.

I like this idea, too.