[LTP] [PATCH] tst_test: Allow to set timeout from test setup()
Jan Stancek
jstancek@redhat.com
Wed Aug 3 16:38:49 CEST 2016
----- Original Message -----
> From: "Cyril Hrubis" <chrubis@suse.cz>
> To: ltp@lists.linux.it
> Cc: "Jan Stancek" <jstancek@redhat.com>
> Sent: Wednesday, 3 August, 2016 3:53:55 PM
> Subject: [PATCH] tst_test: Allow to set timeout from test setup()
>
> There are testcases that take runtime as a parameter. These needs to be
> able to set the timeout dynamically in the test setup().
>
> This commit places the timeout value into the struct results stored in
> shared memory so that it can be changed from the test setup() that runs
> in the child process.
>
> We also reset the alarm() right after we finish setup() so that the new
> value is actually used.
>
> + Added a few lines about test timeouts into the documentation.
>
> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
> doc/test-writing-guidelines.txt | 11 ++++++++++
> include/tst_test.h | 2 ++
> lib/tst_test.c | 45
> +++++++++++++++++++++++++----------------
> 3 files changed, 41 insertions(+), 17 deletions(-)
>
> diff --git a/doc/test-writing-guidelines.txt
> b/doc/test-writing-guidelines.txt
> index db12d18..4990903 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -319,6 +319,10 @@ in range of [0, '.tcnt' - 1].
>
> IMPORTANT: Only one of '.test' and '.test_all' can be set at a time.
>
> +Each test has a default timeout set to 300s. The default timeout can be
> +overriden by setting '.timeout' in the test structure or by calling
> +'tst_set_timeout()' in the test 'setup()'.
> +
> A word about the cleanup() callback
> +++++++++++++++++++++++++++++++++++
>
> @@ -438,6 +442,13 @@ Return the given errno number's corresponding string.
> Using this function to
> translate 'errno' values to strings is preferred. You should not use the
> 'strerror()' function in the testcases.
>
> +[source,c]
> +-------------------------------------------------------------------------------
> +void tst_set_timeout(unsigned int timeout);
> +-------------------------------------------------------------------------------
> +
> +Allows for setting timeout per test iteration dymanically in the test
> setup(),
> +the timeout is specified in seconds.
>
> 2.2.3 Test temporary directory
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> diff --git a/include/tst_test.h b/include/tst_test.h
> index c89e51f..c839178 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -133,6 +133,8 @@ const char *tst_strsig(int sig);
>
> static struct tst_test test;
>
> +void tst_set_timeout(unsigned int timeout);
> +
> int main(int argc, char *argv[])
> {
> tst_run_tcases(argc, argv, &test);
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 26fff97..72d4696 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -46,6 +46,7 @@ struct results {
> int skipped;
> int failed;
> int warnings;
> + unsigned int timeout;
> };
>
> static struct results *results;
> @@ -635,6 +636,8 @@ static void testrun(void)
>
> do_test_setup();
>
> + kill(getppid(), SIGUSR1);
> +
> if (duration > 0)
> stop_time = get_time_ms() + (unsigned long long)(duration * 1000);
>
> @@ -662,7 +665,6 @@ static void testrun(void)
> }
>
> static pid_t test_pid;
> -static unsigned int timeout = 300;
>
> static void alarm_handler(int sig LTP_ATTRIBUTE_UNUSED)
> {
> @@ -671,39 +673,48 @@ static void alarm_handler(int sig LTP_ATTRIBUTE_UNUSED)
>
> static void heartbeat_handler(int sig LTP_ATTRIBUTE_UNUSED)
> {
> - alarm(timeout);
> + alarm(results->timeout);
> }
>
> -void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> +void tst_set_timeout(unsigned int timeout)
> {
> - int status;
> - char *mul;
> -
> - lib_pid = getpid();
> - tst_test = self;
> - TCID = tst_test->tid;
> -
> - do_setup(argc, argv);
> + char *mul = getenv("LTP_TIMEOUT_MUL");
>
> - if (tst_test->timeout)
> - timeout = tst_test->timeout;
> + results->timeout = timeout;
>
> - mul = getenv("LTP_TIMEOUT_MUL");
> if (mul) {
> float m = atof(mul);
>
> if (m < 1)
> tst_brk(TBROK, "Invalid timeout multiplier '%s'", mul);
>
> - timeout = timeout * m + 0.5;
> + results->timeout = results->timeout * m + 0.5;
> }
>
> - tst_res(TINFO, "Timeout per run is %us", timeout);
> + tst_res(TINFO, "Timeout per run is %uh %02um %02us",
> + results->timeout/3600, (results->timeout%3600)/60,
> + results->timeout % 60);
> +}
> +
> +void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
> +{
> + int status;
> +
> + lib_pid = getpid();
> + tst_test = self;
> + TCID = tst_test->tid;
> +
> + do_setup(argc, argv);
> +
> + if (tst_test->timeout)
> + tst_set_timeout(tst_test->timeout);
> + else
> + tst_set_timeout(300);
>
> SAFE_SIGNAL(SIGALRM, alarm_handler);
> SAFE_SIGNAL(SIGUSR1, heartbeat_handler);
>
> - alarm(timeout);
> + alarm(results->timeout);
>
> test_pid = fork();
> if (test_pid < 0)
> --
> 2.7.3
>
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
Looks OK to me, but I'd make tst_set_timeout() do all the work
necessary, if we later change mind and allow to set timeout
in other places. So I'm thinking:
- allow tst_set_timeout() only from test pid
- tst_set_timeout will propagate updates via heartbeat
- timeout is set only once during startup
These are changes that would go on top of your patch (untested):
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 72d46969a51d..a5fcc087a6a5 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -636,7 +636,12 @@ static void testrun(void)
do_test_setup();
- kill(getppid(), SIGUSR1);
+ if (results->timeout)
+ ; /* setup called tst_set_timeout */
+ else if (tst_test->timeout)
+ tst_set_timeout(tst_test->timeout);
+ else
+ tst_set_timeout(300);
if (duration > 0)
stop_time = get_time_ms() + (unsigned long long)(duration * 1000);
@@ -680,6 +685,9 @@ void tst_set_timeout(unsigned int timeout)
{
char *mul = getenv("LTP_TIMEOUT_MUL");
+ if (getpid() == lib_pid)
+ tst_brk(TBROK, "tst_set_timeout called from lib pid");
+
results->timeout = timeout;
if (mul) {
@@ -694,6 +702,7 @@ void tst_set_timeout(unsigned int timeout)
tst_res(TINFO, "Timeout per run is %uh %02um %02us",
results->timeout/3600, (results->timeout%3600)/60,
results->timeout % 60);
+ kill(getppid(), SIGUSR1);
}
void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
@@ -706,16 +715,9 @@ void tst_run_tcases(int argc, char *argv[], struct tst_test *self)
do_setup(argc, argv);
- if (tst_test->timeout)
- tst_set_timeout(tst_test->timeout);
- else
- tst_set_timeout(300);
-
SAFE_SIGNAL(SIGALRM, alarm_handler);
SAFE_SIGNAL(SIGUSR1, heartbeat_handler);
- alarm(results->timeout);
-
test_pid = fork();
if (test_pid < 0)
tst_brk(TBROK | TERRNO, "fork()");
Regards,
Jan
More information about the ltp
mailing list