linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Changes to timers Maefile to use shared logic for run_tests and install
@ 2015-03-13 22:57 Shuah Khan
  2015-03-13 22:57 ` [PATCH 1/2] selftests: add ability to run and emit script for tests args to lib.mk Shuah Khan
  2015-03-13 22:57 ` [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests Shuah Khan
  0 siblings, 2 replies; 10+ messages in thread
From: Shuah Khan @ 2015-03-13 22:57 UTC (permalink / raw)
  To: john.stultz, tglx, mpe; +Cc: Shuah Khan, linux-api, linux-kernel

This patch set:

adds ability to run and emit script for tests args to lib.mk.
lib.mk provides a way to override the default RUN_TESTS and
EMIT_TESTS, however in the cases where a test has several
executables that don't require any special args and/or handling
and some that do, overrding will be cumbersome. Add variables
RUN_TESTS_WITH_ARGS, and EMIT_TESTS_WITH_ARGS to enable a test to
run and exmit script for such special executables.

Timers test Makefile is changed to use the shared logic for running
and installing tests.

Shuah Khan (2):
  selftests: add ability to run and emit script for tests args to lib.mk
  selftests/timers: change to use shared logic to run and install tests

 tools/testing/selftests/lib.mk          |  6 ++++++
 tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

-- 
2.1.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] selftests: add ability to run and emit script for tests args to lib.mk
  2015-03-13 22:57 [PATCH 0/2] Changes to timers Maefile to use shared logic for run_tests and install Shuah Khan
@ 2015-03-13 22:57 ` Shuah Khan
  2015-03-13 22:57 ` [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests Shuah Khan
  1 sibling, 0 replies; 10+ messages in thread
From: Shuah Khan @ 2015-03-13 22:57 UTC (permalink / raw)
  To: john.stultz, tglx, mpe; +Cc: Shuah Khan, linux-api, linux-kernel

Add ability to run, emit, install tests take arguments to the
shared logic in lib.mk. lib.mk provides a way to override the
default RUN_TESTS and EMIT_TESTS, however in the cases where a
test has several executables that don't require any special args
and/or handling and some that do, overrding will be cumbersome.
Add variables RUN_TESTS_WITH_ARGS, and EMIT_TESTS_WITH_ARGS to
enable a test to run and exmit script for such special executables.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/lib.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 7bd3dab..78cdf91 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -6,6 +6,9 @@ endef
 
 run_tests: all
 	$(RUN_TESTS)
+ifdef RUN_TESTS_WITH_ARGS
+	$(RUN_TESTS_WITH_ARGS)
+endif
 
 define INSTALL_RULE
 	mkdir -p $(INSTALL_PATH)
@@ -27,5 +30,8 @@ endef
 
 emit_tests:
 	$(EMIT_TESTS)
+ifdef EMIT_TESTS_WITH_ARGS
+	$(EMIT_TESTS_WITH_ARGS)
+endif
 
 .PHONY: run_tests all clean install emit_tests
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-13 22:57 [PATCH 0/2] Changes to timers Maefile to use shared logic for run_tests and install Shuah Khan
  2015-03-13 22:57 ` [PATCH 1/2] selftests: add ability to run and emit script for tests args to lib.mk Shuah Khan
@ 2015-03-13 22:57 ` Shuah Khan
  2015-03-14  3:14   ` John Stultz
  2015-03-15  8:42   ` Michael Ellerman
  1 sibling, 2 replies; 10+ messages in thread
From: Shuah Khan @ 2015-03-13 22:57 UTC (permalink / raw)
  To: john.stultz, tglx, mpe; +Cc: Shuah Khan, linux-api, linux-kernel

Change the timers Makefile to make use of shared run and install
logic in lib.mk. Destructive tests are installed. Regular tests
are emited to run_kselftest script to match the run_tests behavior.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
index 9da3498..61e7284 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
 	alarmtimer-suspend change_skew skew_consistency clocksource-switch \
 	leap-a-day leapcrash set-tai set-2038
 
+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
+		inconsistency-check raw_skew
+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
+		skew_consistency clocksource-switch leap-a-day leapcrash \
+		set-tai set-2038
+
+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
+
+EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
+
 all: ${bins}
 
 # these are all "safe" tests that don't modify
 # system time or require escalated privledges
-run_tests: all
-	./posix_timers
-	./nanosleep
-	./nsleep-lat
-	./set-timer-lat
-	./mqueue-lat
-	./inconsistency-check
-	./raw_skew
-	./threadtest -t 30 -n 8
+include ../lib.mk
 
 # these tests require escalated privledges
 # and may modify the system time or trigger
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-13 22:57 ` [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests Shuah Khan
@ 2015-03-14  3:14   ` John Stultz
  2015-03-16  2:48     ` Michael Ellerman
  2015-03-18 15:43     ` Shuah Khan
  2015-03-15  8:42   ` Michael Ellerman
  1 sibling, 2 replies; 10+ messages in thread
From: John Stultz @ 2015-03-14  3:14 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Thomas Gleixner, mpe, Linux API, lkml

On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> Change the timers Makefile to make use of shared run and install
> logic in lib.mk. Destructive tests are installed. Regular tests
> are emited to run_kselftest script to match the run_tests behavior.
>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
>  tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
> index 9da3498..61e7284 100644
> --- a/tools/testing/selftests/timers/Makefile
> +++ b/tools/testing/selftests/timers/Makefile
> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
>         alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>         leap-a-day leapcrash set-tai set-2038
>
> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
> +               inconsistency-check raw_skew
> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
> +               skew_consistency clocksource-switch leap-a-day leapcrash \
> +               set-tai set-2038
> +
> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
> +
> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
> +

So my make-foo isn't very strong, but no objections from me.

My only thoughts:
1) Would it be better if threadtest can be made to have better
defaults for kselftest so you don't need that extra logic?
2) While I get that TEST_FILES is likely going to be used to copy the
destructive tests over, It feels a little like its being bundled in
with something like data files that tests might need, which seems sort
of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
make more sense?

thanks
-john

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-13 22:57 ` [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests Shuah Khan
  2015-03-14  3:14   ` John Stultz
@ 2015-03-15  8:42   ` Michael Ellerman
  2015-03-16  2:46     ` Michael Ellerman
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2015-03-15  8:42 UTC (permalink / raw)
  To: Shuah Khan, john.stultz, tglx, mpe; +Cc: linux-api, linux-kernel



On 14 March 2015 09:57:51 GMT+11:00, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>Change the timers Makefile to make use of shared run and install
>logic in lib.mk. Destructive tests are installed. Regular tests
>are emited to run_kselftest script to match the run_tests behavior.
>
>Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>---
> tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
>diff --git a/tools/testing/selftests/timers/Makefile
>b/tools/testing/selftests/timers/Makefile
>index 9da3498..61e7284 100644
>--- a/tools/testing/selftests/timers/Makefile
>+++ b/tools/testing/selftests/timers/Makefile
>@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check
>nsleep-lat raw_skew \
> 	alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> 	leap-a-day leapcrash set-tai set-2038
> 
>+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat
>mqueue-lat \
>+		inconsistency-check raw_skew
>+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew
>\
>+		skew_consistency clocksource-switch leap-a-day leapcrash \
>+		set-tai set-2038
>+
>+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests:
>threadtest [FAIL]"

You shouldn't need this separate variable. As long as you override RUN_TESTS after you include lib.mk you can include the default value, eg:

override RUN_TESTS := $(RUN_TESTS) ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"

I'll test that in the morning and send a proper patch.

cheers
-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-15  8:42   ` Michael Ellerman
@ 2015-03-16  2:46     ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2015-03-16  2:46 UTC (permalink / raw)
  To: Shuah Khan; +Cc: john.stultz, tglx, linux-api, linux-kernel

On Sun, 2015-03-15 at 19:42 +1100, Michael Ellerman wrote:
> 
> On 14 March 2015 09:57:51 GMT+11:00, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> >Change the timers Makefile to make use of shared run and install
> >logic in lib.mk. Destructive tests are installed. Regular tests
> >are emited to run_kselftest script to match the run_tests behavior.
> >
> >Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> >---
> > tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
> > 1 file changed, 11 insertions(+), 9 deletions(-)
> >
> >diff --git a/tools/testing/selftests/timers/Makefile
> >b/tools/testing/selftests/timers/Makefile
> >index 9da3498..61e7284 100644
> >--- a/tools/testing/selftests/timers/Makefile
> >+++ b/tools/testing/selftests/timers/Makefile
> >@@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check
> >nsleep-lat raw_skew \
> > 	alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> > 	leap-a-day leapcrash set-tai set-2038
> > 
> >+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat
> >mqueue-lat \
> >+		inconsistency-check raw_skew
> >+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew
> >\
> >+		skew_consistency clocksource-switch leap-a-day leapcrash \
> >+		set-tai set-2038
> >+
> >+RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests:
> >threadtest [FAIL]"
> 
> You shouldn't need this separate variable. As long as you override RUN_TESTS after you include lib.mk you can include the default value, eg:
> 
> override RUN_TESTS := $(RUN_TESTS) ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
> 
> I'll test that in the morning and send a proper patch.

How's this look?

Contents of install/timers (same as with your patch):

$ ls install/timers/
alarmtimer-suspend*  clocksource-switch*   leap-a-day*  mqueue-lat*  nsleep-lat*    raw_skew*  set-tai*        skew_consistency*  valid-adjtimex*
change_skew*         inconsistency-check*  leapcrash*   nanosleep*   posix_timers*  set-2038*  set-timer-lat*  threadtest*

And in run_kselftest.sh:

echo ; echo Running tests in timers
echo ========================================
cd timers
(./posix_timers && echo "selftests: posix_timers [PASS]") || echo "selftests: posix_timers [FAIL]"
(./nanosleep && echo "selftests: nanosleep [PASS]") || echo "selftests: nanosleep [FAIL]"
(./nsleep-lat && echo "selftests: nsleep-lat [PASS]") || echo "selftests: nsleep-lat [FAIL]"
(./set-timer-lat && echo "selftests: set-timer-lat [PASS]") || echo "selftests: set-timer-lat [FAIL]"
(./mqueue-lat && echo "selftests: mqueue-lat [PASS]") || echo "selftests: mqueue-lat [FAIL]"
(./inconsistency-check && echo "selftests: inconsistency-check [PASS]") || echo "selftests: inconsistency-check [FAIL]"
(./raw_skew && echo "selftests: raw_skew [PASS]") || echo "selftests: raw_skew [FAIL]"
./threadtest -t 30 -n 8 || echo selftests: threadtest [FAIL]
cd $ROOT


cheers



>From 5ea5ea0e5adee5317978965acc4594dce648da63 Mon Sep 17 00:00:00 2001
From: Michael Ellerman <mpe@ellerman.id.au>
Date: Mon, 16 Mar 2015 13:38:06 +1100
Subject: [PATCH] selftests/timers: Use shared logic to run and install tests

Change the timers Makefile to make use of shared run and install logic
in lib.mk. Destructive tests are installed. Regular tests are emited to
run_kselftest script to match the run_tests behavior.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 tools/testing/selftests/timers/Makefile | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
index 9da3498987c8..8ba056ee8b48 100644
--- a/tools/testing/selftests/timers/Makefile
+++ b/tools/testing/selftests/timers/Makefile
@@ -1,25 +1,26 @@
-CC = $(CROSS_COMPILE)gcc
 BUILD_FLAGS = -DKTEST
 CFLAGS += -O3 -Wl,-no-as-needed -Wall $(BUILD_FLAGS)
 LDFLAGS += -lrt -lpthread
-bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
-	set-timer-lat threadtest mqueue-lat valid-adjtimex \
-	alarmtimer-suspend change_skew skew_consistency clocksource-switch \
-	leap-a-day leapcrash set-tai set-2038
-
-all: ${bins}
 
 # these are all "safe" tests that don't modify
 # system time or require escalated privledges
-run_tests: all
-	./posix_timers
-	./nanosleep
-	./nsleep-lat
-	./set-timer-lat
-	./mqueue-lat
-	./inconsistency-check
-	./raw_skew
-	./threadtest -t 30 -n 8
+TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
+		inconsistency-check raw_skew
+
+TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
+		skew_consistency clocksource-switch leap-a-day leapcrash \
+		set-tai set-2038
+
+bins = $(TEST_PROGS) $(TEST_FILES)
+
+all: ${bins}
+
+include ../lib.mk
+
+THREADTEST := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
+
+override RUN_TESTS := $(RUN_TESTS) $(THREADTEST)
+override EMIT_TESTS := $(EMIT_TESTS) echo "$(THREADTEST)"
 
 # these tests require escalated privledges
 # and may modify the system time or trigger
-- 
2.1.0






^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-14  3:14   ` John Stultz
@ 2015-03-16  2:48     ` Michael Ellerman
  2015-03-18 15:55       ` Shuah Khan
  2015-03-18 15:43     ` Shuah Khan
  1 sibling, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2015-03-16  2:48 UTC (permalink / raw)
  To: John Stultz; +Cc: Shuah Khan, Thomas Gleixner, Linux API, lkml

On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> > Change the timers Makefile to make use of shared run and install
> > logic in lib.mk. Destructive tests are installed. Regular tests
> > are emited to run_kselftest script to match the run_tests behavior.
> >
> > Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> > ---
> >  tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
> >  1 file changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
> > index 9da3498..61e7284 100644
> > --- a/tools/testing/selftests/timers/Makefile
> > +++ b/tools/testing/selftests/timers/Makefile
> > @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
> >         alarmtimer-suspend change_skew skew_consistency clocksource-switch \
> >         leap-a-day leapcrash set-tai set-2038
> >
> > +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
> > +               inconsistency-check raw_skew
> > +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
> > +               skew_consistency clocksource-switch leap-a-day leapcrash \
> > +               set-tai set-2038
> > +
> > +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
> > +
> > +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
> > +
> 
> So my make-foo isn't very strong, but no objections from me.
> 
> My only thoughts:
> 1) Would it be better if threadtest can be made to have better
> defaults for kselftest so you don't need that extra logic?

That would help. But with the patch I just sent I think it's no bother, it's
only a little extra logic and it's only in the timers Makefile.

> 2) While I get that TEST_FILES is likely going to be used to copy the
> destructive tests over, It feels a little like its being bundled in
> with something like data files that tests might need, which seems sort
> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> make more sense?

That doesn't really bother me. You're right that TEST_FILES is originally
intended for data files etc. but I don't think it's a big hack to use it for
other tests that shouldn't be run by default. Still if it bothers you I'm happy
to add a separate variable for it, they are cheap :)

cheers



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-14  3:14   ` John Stultz
  2015-03-16  2:48     ` Michael Ellerman
@ 2015-03-18 15:43     ` Shuah Khan
  1 sibling, 0 replies; 10+ messages in thread
From: Shuah Khan @ 2015-03-18 15:43 UTC (permalink / raw)
  To: John Stultz; +Cc: Thomas Gleixner, mpe, Linux API, lkml, Shuah Khan

On 03/13/2015 09:14 PM, John Stultz wrote:
> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> Change the timers Makefile to make use of shared run and install
>> logic in lib.mk. Destructive tests are installed. Regular tests
>> are emited to run_kselftest script to match the run_tests behavior.
>>
>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>> ---
>>  tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>> index 9da3498..61e7284 100644
>> --- a/tools/testing/selftests/timers/Makefile
>> +++ b/tools/testing/selftests/timers/Makefile
>> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
>>         alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>>         leap-a-day leapcrash set-tai set-2038
>>
>> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
>> +               inconsistency-check raw_skew
>> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
>> +               skew_consistency clocksource-switch leap-a-day leapcrash \
>> +               set-tai set-2038
>> +
>> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
>> +
>> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
>> +
> 
> So my make-foo isn't very strong, but no objections from me.
> 
> My only thoughts:
> 1) Would it be better if threadtest can be made to have better
> defaults for kselftest so you don't need that extra logic?

Yes. If you can change the threadset to run with -t 30 -n 8
in default case, that would make the logic simpler. Can you
send me a patch to do this?


> 2) While I get that TEST_FILES is likely going to be used to copy the
> destructive tests over, It feels a little like its being bundled in
> with something like data files that tests might need, which seems sort
> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> make more sense?

I agree with you on this. TEST_FILES usage is an overload.

-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-16  2:48     ` Michael Ellerman
@ 2015-03-18 15:55       ` Shuah Khan
  2015-03-19  0:21         ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Shuah Khan @ 2015-03-18 15:55 UTC (permalink / raw)
  To: Michael Ellerman, John Stultz
  Cc: Thomas Gleixner, Linux API, lkml, Shuah Khan

On 03/15/2015 08:48 PM, Michael Ellerman wrote:
> On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
>> On Fri, Mar 13, 2015 at 3:57 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>>> Change the timers Makefile to make use of shared run and install
>>> logic in lib.mk. Destructive tests are installed. Regular tests
>>> are emited to run_kselftest script to match the run_tests behavior.
>>>
>>> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
>>> ---
>>>  tools/testing/selftests/timers/Makefile | 20 +++++++++++---------
>>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/timers/Makefile b/tools/testing/selftests/timers/Makefile
>>> index 9da3498..61e7284 100644
>>> --- a/tools/testing/selftests/timers/Makefile
>>> +++ b/tools/testing/selftests/timers/Makefile
>>> @@ -7,19 +7,21 @@ bins = posix_timers nanosleep inconsistency-check nsleep-lat raw_skew \
>>>         alarmtimer-suspend change_skew skew_consistency clocksource-switch \
>>>         leap-a-day leapcrash set-tai set-2038
>>>
>>> +TEST_PROGS = posix_timers nanosleep nsleep-lat set-timer-lat mqueue-lat \
>>> +               inconsistency-check raw_skew
>>> +TEST_FILES = threadtest alarmtimer-suspend valid-adjtimex change_skew \
>>> +               skew_consistency clocksource-switch leap-a-day leapcrash \
>>> +               set-tai set-2038
>>> +
>>> +RUN_TESTS_WITH_ARGS := ./threadtest -t 30 -n 8 || echo "selftests: threadtest [FAIL]"
>>> +
>>> +EMIT_TESTS_WITH_ARGS := echo "$(RUN_TESTS_WITH_ARGS)"
>>> +
>>
>> So my make-foo isn't very strong, but no objections from me.
>>
>> My only thoughts:
>> 1) Would it be better if threadtest can be made to have better
>> defaults for kselftest so you don't need that extra logic?
> 
> That would help. But with the patch I just sent I think it's no bother, it's
> only a little extra logic and it's only in the timers Makefile.

Let's go with a threadtest patch with better defaults. It will emit
scripts logic as well. Awesome. I just saw John's new patch in my
Inbox. Thanks.

> 
>> 2) While I get that TEST_FILES is likely going to be used to copy the
>> destructive tests over, It feels a little like its being bundled in
>> with something like data files that tests might need, which seems sort
>> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
>> make more sense?
> 
> That doesn't really bother me. You're right that TEST_FILES is originally
> intended for data files etc. but I don't think it's a big hack to use it for
> other tests that shouldn't be run by default. Still if it bothers you I'm happy
> to add a separate variable for it, they are cheap :)
> 

Michael,

Could you please make the change from TEST_FILES to TEST_PROGS_EXTENDED
which is definitely better than overloading TEST_FILES?

I think this would probably only change Add Install target patch. You
can send me patch v5 with that change and I will override the next with
your new one. Thanks for doing this.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests
  2015-03-18 15:55       ` Shuah Khan
@ 2015-03-19  0:21         ` Michael Ellerman
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Ellerman @ 2015-03-19  0:21 UTC (permalink / raw)
  To: Shuah Khan; +Cc: John Stultz, Thomas Gleixner, Linux API, lkml

On Wed, 2015-03-18 at 09:55 -0600, Shuah Khan wrote:
> On 03/15/2015 08:48 PM, Michael Ellerman wrote:
> > On Fri, 2015-03-13 at 20:14 -0700, John Stultz wrote:
> >> My only thoughts:
> >> 1) Would it be better if threadtest can be made to have better
> >> defaults for kselftest so you don't need that extra logic?
> > 
> > That would help. But with the patch I just sent I think it's no bother, it's
> > only a little extra logic and it's only in the timers Makefile.
> 
> Let's go with a threadtest patch with better defaults. It will emit
> scripts logic as well. Awesome. I just saw John's new patch in my
> Inbox. Thanks.

Fine by me.

> >> 2) While I get that TEST_FILES is likely going to be used to copy the
> >> destructive tests over, It feels a little like its being bundled in
> >> with something like data files that tests might need, which seems sort
> >> of hackish. Would TEST_PROGS_EXTENDED or something be more clear and
> >> make more sense?
> > 
> > That doesn't really bother me. You're right that TEST_FILES is originally
> > intended for data files etc. but I don't think it's a big hack to use it for
> > other tests that shouldn't be run by default. Still if it bothers you I'm happy
> > to add a separate variable for it, they are cheap :)
> 
> Could you please make the change from TEST_FILES to TEST_PROGS_EXTENDED
> which is definitely better than overloading TEST_FILES?

I don't see the point, but I don't care that much.

> I think this would probably only change Add Install target patch. You
> can send me patch v5 with that change and I will override the next with
> your new one. Thanks for doing this.

You shouldn't rebase your next branch, people may have already merged it (like
me). You should apply it as an additional patch on top.

Patch sent.

cheers



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-03-19  0:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 22:57 [PATCH 0/2] Changes to timers Maefile to use shared logic for run_tests and install Shuah Khan
2015-03-13 22:57 ` [PATCH 1/2] selftests: add ability to run and emit script for tests args to lib.mk Shuah Khan
2015-03-13 22:57 ` [PATCH 2/2] selftests/timers: change to use shared logic to run and install tests Shuah Khan
2015-03-14  3:14   ` John Stultz
2015-03-16  2:48     ` Michael Ellerman
2015-03-18 15:55       ` Shuah Khan
2015-03-19  0:21         ` Michael Ellerman
2015-03-18 15:43     ` Shuah Khan
2015-03-15  8:42   ` Michael Ellerman
2015-03-16  2:46     ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).