v2:
- update documentation
- include SPDX line in extracted script
v1: https://lore.kernel.org/linux-kselftest/[email protected]/
Hi!
I really like Hangbin Liu's intent[1] but I think we need to be a little
more clean about the implementation. This extracts run_kselftest.sh from
the Makefile so it can actually be changed without embeds, etc. Instead,
generate the test list into a text file. Everything gets much simpler.
:)
And in patch 2, I add back Hangbin Liu's new options (with some extra
added) with knowledge of "collections" (i.e. Makefile TARGETS) and
subtests. This should work really well with LAVA too, which needs to
manipulate the lists of tests being run.
Thoughts?
-Kees
[1] https://lore.kernel.org/lkml/[email protected]/
Kees Cook (3):
selftests: Extract run_kselftest.sh and generate stand-alone test list
selftests/run_kselftest.sh: Make each test individually selectable
doc: dev-tools: kselftest.rst: Update examples and paths
Documentation/dev-tools/kselftest.rst | 35 +++++----
tools/testing/selftests/Makefile | 26 ++-----
tools/testing/selftests/lib.mk | 5 +-
tools/testing/selftests/run_kselftest.sh | 93 ++++++++++++++++++++++++
4 files changed, 124 insertions(+), 35 deletions(-)
create mode 100755 tools/testing/selftests/run_kselftest.sh
--
2.25.1
Instead of building a script on the fly (which just repeats the same
thing for each test collection), move the script out of the Makefile and
into run_kselftest.sh, which reads kselftest-list.txt.
Adjust the emit_tests target to report each test on a separate line so
that test running tools (e.g. LAVA) can easily remove individual
tests (for example, as seen in [1]).
[1] https://github.com/Linaro/test-definitions/pull/208/commits/2e7b62155e4998e54ac0587704932484d4ff84c8
Signed-off-by: Kees Cook <[email protected]>
---
tools/testing/selftests/Makefile | 26 ++++++----------------
tools/testing/selftests/lib.mk | 5 ++---
tools/testing/selftests/run_kselftest.sh | 28 ++++++++++++++++++++++++
3 files changed, 37 insertions(+), 22 deletions(-)
create mode 100755 tools/testing/selftests/run_kselftest.sh
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 15c1c1359c50..d9c283503159 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -206,6 +206,7 @@ KSFT_INSTALL_PATH := $(abspath $(KSFT_INSTALL_PATH))
# Avoid changing the rest of the logic here and lib.mk.
INSTALL_PATH := $(KSFT_INSTALL_PATH)
ALL_SCRIPT := $(INSTALL_PATH)/run_kselftest.sh
+TEST_LIST := $(INSTALL_PATH)/kselftest-list.txt
install: all
ifdef INSTALL_PATH
@@ -214,6 +215,8 @@ ifdef INSTALL_PATH
install -m 744 kselftest/module.sh $(INSTALL_PATH)/kselftest/
install -m 744 kselftest/runner.sh $(INSTALL_PATH)/kselftest/
install -m 744 kselftest/prefix.pl $(INSTALL_PATH)/kselftest/
+ install -m 744 run_kselftest.sh $(INSTALL_PATH)/
+ rm -f $(TEST_LIST)
@ret=1; \
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
@@ -222,33 +225,18 @@ ifdef INSTALL_PATH
ret=$$((ret * $$?)); \
done; exit $$ret;
- @# Ask all targets to emit their test scripts
- echo "#!/bin/sh" > $(ALL_SCRIPT)
- echo "BASE_DIR=\$$(realpath \$$(dirname \$$0))" >> $(ALL_SCRIPT)
- echo "cd \$$BASE_DIR" >> $(ALL_SCRIPT)
- echo ". ./kselftest/runner.sh" >> $(ALL_SCRIPT)
- echo "ROOT=\$$PWD" >> $(ALL_SCRIPT)
- echo "if [ \"\$$1\" = \"--summary\" ]; then" >> $(ALL_SCRIPT)
- echo " logfile=\$$BASE_DIR/output.log" >> $(ALL_SCRIPT)
- echo " cat /dev/null > \$$logfile" >> $(ALL_SCRIPT)
- echo "fi" >> $(ALL_SCRIPT)
- @# While building run_kselftest.sh skip also non-existent TARGET dirs:
+ @# Ask all targets to emit their test scripts
+ @# While building kselftest-list.text skip also non-existent TARGET dirs:
@# they could be the result of a build failure and should NOT be
@# included in the generated runlist.
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
[ ! -d $(INSTALL_PATH)/$$TARGET ] && echo "Skipping non-existent dir: $$TARGET" && continue; \
- echo "[ -w /dev/kmsg ] && echo \"kselftest: Running tests in $$TARGET\" >> /dev/kmsg" >> $(ALL_SCRIPT); \
- echo "cd $$TARGET" >> $(ALL_SCRIPT); \
- echo -n "run_many" >> $(ALL_SCRIPT); \
echo -n "Emit Tests for $$TARGET\n"; \
- $(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET -C $$TARGET emit_tests >> $(ALL_SCRIPT); \
- echo "" >> $(ALL_SCRIPT); \
- echo "cd \$$ROOT" >> $(ALL_SCRIPT); \
+ $(MAKE) -s --no-print-directory OUTPUT=$$BUILD_TARGET COLLECTION=$$TARGET \
+ -C $$TARGET emit_tests >> $(TEST_LIST); \
done;
-
- chmod u+x $(ALL_SCRIPT)
else
$(error Error: set INSTALL_PATH to use install)
endif
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 51124b962d56..30848ca36555 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -107,9 +107,8 @@ endif
emit_tests:
for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \
BASENAME_TEST=`basename $$TEST`; \
- echo " \\"; \
- echo -n " \"$$BASENAME_TEST\""; \
- done; \
+ echo "$(COLLECTION):$$BASENAME_TEST"; \
+ done
# define if isn't already. It is undefined in make O= case.
ifeq ($(RM),)
diff --git a/tools/testing/selftests/run_kselftest.sh b/tools/testing/selftests/run_kselftest.sh
new file mode 100755
index 000000000000..8b0ad4766d78
--- /dev/null
+++ b/tools/testing/selftests/run_kselftest.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Run installed kselftest tests.
+#
+BASE_DIR=$(realpath $(dirname $0))
+cd $BASE_DIR
+TESTS="$BASE_DIR"/kselftest-list.txt
+if [ ! -r "$TESTS" ] ; then
+ echo "$0: Could not find list of tests to run ($TESTS)" >&2
+ exit 1
+fi
+available="$(cat "$TESTS")"
+
+. ./kselftest/runner.sh
+ROOT=$PWD
+
+if [ "$1" = "--summary" ] ; then
+ logfile="$BASE_DIR"/output.log
+ cat /dev/null > $logfile
+fi
+
+collections=$(echo "$available" | cut -d: -f1 | uniq)
+for collection in $collections ; do
+ [ -w /dev/kmsg ] && echo "kselftest: Running tests in $collection" >> /dev/kmsg
+ tests=$(echo "$available" | grep "^$collection:" | cut -d: -f2)
+ (cd "$collection" && run_many $tests)
+done
--
2.25.1
On Mon, Sep 28, 2020 at 01:26:47PM -0700, Kees Cook wrote:
> v2:
> - update documentation
> - include SPDX line in extracted script
> v1: https://lore.kernel.org/linux-kselftest/[email protected]/
>
I'm not sure if the doc update are all appropriate. Need others help review.
The script part looks good to me. Thanks for your update.
Regards
Hangbin
On Tue, 29 Sep 2020 at 01:56, Kees Cook <[email protected]> wrote:
>
> v2:
> - update documentation
> - include SPDX line in extracted script
> v1: https://lore.kernel.org/linux-kselftest/[email protected]/
>
>
> Hi!
>
> I really like Hangbin Liu's intent[1] but I think we need to be a little
> more clean about the implementation. This extracts run_kselftest.sh from
> the Makefile so it can actually be changed without embeds, etc. Instead,
> generate the test list into a text file. Everything gets much simpler.
> :)
>
> And in patch 2, I add back Hangbin Liu's new options (with some extra
> added) with knowledge of "collections" (i.e. Makefile TARGETS) and
> subtests. This should work really well with LAVA too, which needs to
> manipulate the lists of tests being run.
>
> Thoughts?
I have tested this patch set on LAVA with full run and it went well.
>
> -Kees
>
> [1] https://lore.kernel.org/lkml/[email protected]/
>
> Kees Cook (3):
> selftests: Extract run_kselftest.sh and generate stand-alone test list
> selftests/run_kselftest.sh: Make each test individually selectable
> doc: dev-tools: kselftest.rst: Update examples and paths
>
> Documentation/dev-tools/kselftest.rst | 35 +++++----
> tools/testing/selftests/Makefile | 26 ++-----
> tools/testing/selftests/lib.mk | 5 +-
> tools/testing/selftests/run_kselftest.sh | 93 ++++++++++++++++++++++++
> 4 files changed, 124 insertions(+), 35 deletions(-)
> create mode 100755 tools/testing/selftests/run_kselftest.sh
>
> --
> 2.25.1
>
- Naresh
On Wed, Sep 30, 2020 at 09:47:49PM +0530, Naresh Kamboju wrote:
> On Tue, 29 Sep 2020 at 01:56, Kees Cook <[email protected]> wrote:
> >
> > v2:
> > - update documentation
> > - include SPDX line in extracted script
> > v1: https://lore.kernel.org/linux-kselftest/[email protected]/
> >
> >
> > Hi!
> >
> > I really like Hangbin Liu's intent[1] but I think we need to be a little
> > more clean about the implementation. This extracts run_kselftest.sh from
> > the Makefile so it can actually be changed without embeds, etc. Instead,
> > generate the test list into a text file. Everything gets much simpler.
> > :)
> >
> > And in patch 2, I add back Hangbin Liu's new options (with some extra
> > added) with knowledge of "collections" (i.e. Makefile TARGETS) and
> > subtests. This should work really well with LAVA too, which needs to
> > manipulate the lists of tests being run.
> >
> > Thoughts?
>
> I have tested this patch set on LAVA with full run and it went well.
Thank you! You can include this as a tag too, so a "b4 am" will pick it
up:
Tested-by: Naresh Kamboju <[email protected]>
-Kees
>
> >
> > -Kees
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> >
> > Kees Cook (3):
> > selftests: Extract run_kselftest.sh and generate stand-alone test list
> > selftests/run_kselftest.sh: Make each test individually selectable
> > doc: dev-tools: kselftest.rst: Update examples and paths
> >
> > Documentation/dev-tools/kselftest.rst | 35 +++++----
> > tools/testing/selftests/Makefile | 26 ++-----
> > tools/testing/selftests/lib.mk | 5 +-
> > tools/testing/selftests/run_kselftest.sh | 93 ++++++++++++++++++++++++
> > 4 files changed, 124 insertions(+), 35 deletions(-)
> > create mode 100755 tools/testing/selftests/run_kselftest.sh
> >
> > --
> > 2.25.1
> >
>
> - Naresh
--
Kees Cook
On Tue, 29 Sep 2020 at 01:56, Kees Cook <[email protected]> wrote:
>
> Instead of building a script on the fly (which just repeats the same
> thing for each test collection), move the script out of the Makefile and
> into run_kselftest.sh, which reads kselftest-list.txt.
>
> Adjust the emit_tests target to report each test on a separate line so
> that test running tools (e.g. LAVA) can easily remove individual
> tests (for example, as seen in [1]).
>
> [1] https://github.com/Linaro/test-definitions/pull/208/commits/2e7b62155e4998e54ac0587704932484d4ff84c8
>
> Signed-off-by: Kees Cook <[email protected]>
Tested-by: Naresh Kamboju <[email protected]>
- Naresh