[PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[Shuah Khan]

When a test exits with skip exit code of 4, "make run_destructive_tests"
halts testing. Fix run_destructive_tests target to handle error exit codes.

Reported-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
index c805ab048d26..6c1327278d5f 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
 
 include ../lib.mk
 
+define RUN_DESTRUCTIVE_TESTS
+	@for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
+		BASENAME_TEST=`basename $$TEST`;	\
+		if [ ! -x $$BASENAME_TEST ]; then	\
+			echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
+			echo "selftests: $$BASENAME_TEST [FAIL]"; \
+		else					\
+			cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
+		fi;					\
+	done;
+endef
+
 # these tests require escalated privileges
 # and may modify the system time or trigger
 # other behavior like suspend
 run_destructive_tests: run_tests
-	./alarmtimer-suspend
-	./valid-adjtimex
-	./adjtick
-	./change_skew
-	./skew_consistency
-	./clocksource-switch
-	./freq-step
-	./leap-a-day -s -i 10
-	./leapcrash
-	./set-tz
-	./set-tai
-	./set-2038
-
+	$(RUN_DESTRUCTIVE_TESTS)
-- 
2.11.0

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[John Stultz]

On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> When a test exits with skip exit code of 4, "make run_destructive_tests"
> halts testing. Fix run_destructive_tests target to handle error exit codes.
>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
> index c805ab048d26..6c1327278d5f 100644
> --- a/tools/testing/selftests/timers/Makefile
> +++ b/tools/testing/selftests/timers/Makefile
> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>
>  include ../lib.mk
>
> +define RUN_DESTRUCTIVE_TESTS
> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
> +               BASENAME_TEST=`basename $$TEST`;        \
> +               if [ ! -x $$BASENAME_TEST ]; then       \
> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
> +               else                                    \
> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
> +               fi;                                     \
> +       done;
> +endef
> +
>  # these tests require escalated privileges
>  # and may modify the system time or trigger
>  # other behavior like suspend
>  run_destructive_tests: run_tests
> -       ./alarmtimer-suspend
> -       ./valid-adjtimex
> -       ./adjtick
> -       ./change_skew
> -       ./skew_consistency
> -       ./clocksource-switch
> -       ./freq-step
> -       ./leap-a-day -s -i 10


So this resolves the issue with tests returning skipped, but we'll
need to change the leap-a-day test to default to the specified
arguments above, as it runs indefinitely w/o arguments. Thus with the
patch above, make run_destructive_tests never completes.

I'll scratch a patch out to make the change in defaults.

thanks
-john

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[Shuah Khan]

On 08/18/2017 04:13 PM, John Stultz wrote:
> On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> When a test exits with skip exit code of 4, "make run_destructive_tests"
>> halts testing. Fix run_destructive_tests target to handle error exit codes.
>>
>> Reported-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>> index c805ab048d26..6c1327278d5f 100644
>> --- a/tools/testing/selftests/timers/Makefile
>> +++ b/tools/testing/selftests/timers/Makefile
>> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>>
>>  include ../lib.mk
>>
>> +define RUN_DESTRUCTIVE_TESTS
>> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
>> +               BASENAME_TEST=`basename $$TEST`;        \
>> +               if [ ! -x $$BASENAME_TEST ]; then       \
>> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
>> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
>> +               else                                    \
>> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
>> +               fi;                                     \
>> +       done;
>> +endef
>> +
>>  # these tests require escalated privileges
>>  # and may modify the system time or trigger
>>  # other behavior like suspend
>>  run_destructive_tests: run_tests
>> -       ./alarmtimer-suspend
>> -       ./valid-adjtimex
>> -       ./adjtick
>> -       ./change_skew
>> -       ./skew_consistency
>> -       ./clocksource-switch
>> -       ./freq-step
>> -       ./leap-a-day -s -i 10
> 
> 
> So this resolves the issue with tests returning skipped, but we'll
> need to change the leap-a-day test to default to the specified
> arguments above, as it runs indefinitely w/o arguments. Thus with the
> patch above, make run_destructive_tests never completes.

Thanks for catching the regression running leap-a-day indefinitely.
I missed that one :(

> 
> I'll scratch a patch out to make the change in defaults.

That will be great. Thanks,

-- Shuah

[RFC][PATCH] kselftests: timers: leap-a-day: Change default arguments to help test runs

[John Stultz]

Change default arguments for leap-a-day to always set the time
each iteration (rather then waiting for midnight UTC), and to
only run 10 interations (rather then infinite).

If one wants to wait for midnight UTC, they can use the new -w
flag, and we add a note to the argument help that -i -1 will
run infinitely.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <stephen.boyd@linaro.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 tools/testing/selftests/timers/leap-a-day.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/timers/leap-a-day.c b/tools/testing/selftests/timers/leap-a-day.c
index fb46ad6..0670176 100644
--- a/tools/testing/selftests/timers/leap-a-day.c
+++ b/tools/testing/selftests/timers/leap-a-day.c
@@ -190,18 +190,18 @@ int main(int argc, char **argv)
 	struct sigevent se;
 	struct sigaction act;
 	int signum = SIGRTMAX;
-	int settime = 0;
+	int settime = 1;
 	int tai_time = 0;
 	int insert = 1;
-	int iterations = -1;
+	int iterations = 10;
 	int opt;
 
 	/* Process arguments */
 	while ((opt = getopt(argc, argv, "sti:")) != -1) {
 		switch (opt) {
-		case 's':
-			printf("Setting time to speed up testing\n");
-			settime = 1;
+		case 'w':
+			printf("Only setting leap-flag, not changing time. It could take up to a day for leap to trigger.\n");
+			settime = 0;
 			break;
 		case 'i':
 			iterations = atoi(optarg);
@@ -210,9 +210,10 @@ int main(int argc, char **argv)
 			tai_time = 1;
 			break;
 		default:
-			printf("Usage: %s [-s] [-i <iterations>]\n", argv[0]);
-			printf("	-s: Set time to right before leap second each iteration\n");
-			printf("	-i: Number of iterations\n");
+			printf("Usage: %s [-w] [-i <iterations>]\n", argv[0]);
+			printf("	-w: Set flag and wait for leap second each iteration");
+			printf("	    (default sets time to right before leapsecond)\n");
+			printf("	-i: Number of iterations (-1 = infinite, default is 10)\n");
 			printf("	-t: Print TAI time\n");
 			exit(-1);
 		}
-- 
2.7.4

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[John Stultz]

On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> When a test exits with skip exit code of 4, "make run_destructive_tests"
> halts testing. Fix run_destructive_tests target to handle error exit codes.
>
> Reported-by: John Stultz <john.stultz@linaro.org>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
> index c805ab048d26..6c1327278d5f 100644
> --- a/tools/testing/selftests/timers/Makefile
> +++ b/tools/testing/selftests/timers/Makefile
> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>
>  include ../lib.mk
>
> +define RUN_DESTRUCTIVE_TESTS
> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
> +               BASENAME_TEST=`basename $$TEST`;        \
> +               if [ ! -x $$BASENAME_TEST ]; then       \
> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
> +               else                                    \
> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
> +               fi;                                     \
> +       done;
> +endef

One more on this...  you might remove rtctest_setdate from the
TEST_GEN_PROGS_EXTENDED list, since it too requires arguments to test,
and it wasn't a part of run_destructive_tests previously.

I see Benjamin added this test not long ago, but I'm not sure exactly
how he expects it to be run (just manually?).

thanks
-john

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[Benjamin Gaignard]

2017-08-19 2:36 GMT+02:00 John Stultz <john.stultz@linaro.org>:
> On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> When a test exits with skip exit code of 4, "make run_destructive_tests"
>> halts testing. Fix run_destructive_tests target to handle error exit codes.
>>
>> Reported-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>> index c805ab048d26..6c1327278d5f 100644
>> --- a/tools/testing/selftests/timers/Makefile
>> +++ b/tools/testing/selftests/timers/Makefile
>> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>>
>>  include ../lib.mk
>>
>> +define RUN_DESTRUCTIVE_TESTS
>> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
>> +               BASENAME_TEST=`basename $$TEST`;        \
>> +               if [ ! -x $$BASENAME_TEST ]; then       \
>> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
>> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
>> +               else                                    \
>> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
>> +               fi;                                     \
>> +       done;
>> +endef
>
> One more on this...  you might remove rtctest_setdate from the
> TEST_GEN_PROGS_EXTENDED list, since it too requires arguments to test,
> and it wasn't a part of run_destructive_tests previously.
>
> I see Benjamin added this test not long ago, but I'm not sure exactly
> how he expects it to be run (just manually?).

Yes run this test manually is enough

>
> thanks
> -john



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[Shuah Khan]

On 08/20/2017 07:08 AM, Benjamin Gaignard wrote:
> 2017-08-19 2:36 GMT+02:00 John Stultz <john.stultz@linaro.org>:
>> On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> When a test exits with skip exit code of 4, "make run_destructive_tests"
>>> halts testing. Fix run_destructive_tests target to handle error exit codes.
>>>
>>> Reported-by: John Stultz <john.stultz@linaro.org>
>>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>>> ---
>>>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>>> index c805ab048d26..6c1327278d5f 100644
>>> --- a/tools/testing/selftests/timers/Makefile
>>> +++ b/tools/testing/selftests/timers/Makefile
>>> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>>>
>>>  include ../lib.mk
>>>
>>> +define RUN_DESTRUCTIVE_TESTS
>>> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
>>> +               BASENAME_TEST=`basename $$TEST`;        \
>>> +               if [ ! -x $$BASENAME_TEST ]; then       \
>>> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
>>> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
>>> +               else                                    \
>>> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
>>> +               fi;                                     \
>>> +       done;
>>> +endef
>>
>> One more on this...  you might remove rtctest_setdate from the
>> TEST_GEN_PROGS_EXTENDED list, since it too requires arguments to test,
>> and it wasn't a part of run_destructive_tests previously.
>>
>> I see Benjamin added this test not long ago, but I'm not sure exactly
>> how he expects it to be run (just manually?).
> 
> Yes run this test manually is enough
> 

Thanks I will make the change so that rtctest_setdate isn't included in the
destructive tests target and stays in the install so it gets installed on the
target test system. Will send the patch to you both.

thanks,
-- Shuah

Re: [PATCH] selftests: timers: Fix run_destructive_tests target to handle skipped tests

[Shuah Khan]

On 08/18/2017 04:13 PM, John Stultz wrote:
> On Thu, Aug 17, 2017 at 3:48 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> When a test exits with skip exit code of 4, "make run_destructive_tests"
>> halts testing. Fix run_destructive_tests target to handle error exit codes.
>>
>> Reported-by: John Stultz <john.stultz@linaro.org>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>  tools/testing/selftests/timers/Makefile | 26 +++++++++++++-------------
>>  1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>> index c805ab048d26..6c1327278d5f 100644
>> --- a/tools/testing/selftests/timers/Makefile
>> +++ b/tools/testing/selftests/timers/Makefile
>> @@ -13,20 +13,20 @@ TEST_GEN_PROGS_EXTENDED = alarmtimer-suspend valid-adjtimex adjtick change_skew
>>
>>  include ../lib.mk
>>
>> +define RUN_DESTRUCTIVE_TESTS
>> +       @for TEST in $(TEST_GEN_PROGS_EXTENDED); do \
>> +               BASENAME_TEST=`basename $$TEST`;        \
>> +               if [ ! -x $$BASENAME_TEST ]; then       \
>> +                       echo "selftests: Warning: file $$BASENAME_TEST is not executable, correct this.";\
>> +                       echo "selftests: $$BASENAME_TEST [FAIL]"; \
>> +               else                                    \
>> +                       cd `dirname $$TEST`; (./$$BASENAME_TEST && echo "selftests: $$BASENAME_TEST [PASS]") || echo "selftests:  $$BASENAME_TEST [FAIL]"; cd -;\
>> +               fi;                                     \
>> +       done;
>> +endef
>> +
>>  # these tests require escalated privileges
>>  # and may modify the system time or trigger
>>  # other behavior like suspend
>>  run_destructive_tests: run_tests
>> -       ./alarmtimer-suspend
>> -       ./valid-adjtimex
>> -       ./adjtick
>> -       ./change_skew
>> -       ./skew_consistency
>> -       ./clocksource-switch
>> -       ./freq-step
>> -       ./leap-a-day -s -i 10
> 
> 
> So this resolves the issue with tests returning skipped, but we'll
> need to change the leap-a-day test to default to the specified
> arguments above, as it runs indefinitely w/o arguments. Thus with the
> patch above, make run_destructive_tests never completes.
> 
> I'll scratch a patch out to make the change in defaults.
> 
Applied the patch to linux=kselftest for 4.14-rc1 after applying
the leap-a-day test default args change patch. Both are marked for
stable 4.13+

thanks,
-- Shuah

Re: [RFC][PATCH] kselftests: timers: leap-a-day: Change default arguments to help test runs

[Shuah Khan]

On 08/18/2017 05:23 PM, John Stultz wrote:
> Change default arguments for leap-a-day to always set the time
> each iteration (rather then waiting for midnight UTC), and to
> only run 10 interations (rather then infinite).
> 
> If one wants to wait for midnight UTC, they can use the new -w
> flag, and we add a note to the argument help that -i -1 will
> run infinitely.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Miroslav Lichvar <mlichvar@redhat.com>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Prarit Bhargava <prarit@redhat.com>
> Cc: Stephen Boyd <stephen.boyd@linaro.org>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: linux-kselftest@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---

Applied linux-kselftest next for 4.14-rc1. Marked it for stable 4.13+
This patch is applied before the

selftests: timers: Fix run_destructive_tests target to handle skipped tests


thanks,
-- Shuah