Willem de Bruijn wrote:
Jakub Kicinski wrote:
On Fri, 30 Aug 2024 14:47:43 -0400 Willem de Bruijn wrote:
We have directories in net/lib, and it's a target, and it works, no?
net/lib is not a TARGET in tools/testing/selftests/Makefile. Its Makefile only generates dependencies for other targets: TEST_FILES, TEST_GEN_FILES and TEST_INCLUDES.
Oh right, TEST_FILES vs TEST_INCLUDES :(
Looks like only x86 does some weird stuff and prepends $(OUTPUT) to all test names. Otherwise the only TEST_NAME with a / in it is
x86_64/nx_huge_pages_test.sh
Oh interesting precedent. Let me take a look.
But then again maybe we should give up on the idea of using directories? Use some separator like --, I mean:
mv packetdrill/tcp/inq/client.pkt packetdrill/tcp--inq--client.pkt
Assuming we're moving forward with the interpreter idea we don't need directories for multi-threading, just for organization. Which perhaps isn't worth the time investment? Given that we'd mostly interact with these tests via UI which will flatten it all back?
That's definitely simpler :)
I'd like to keep diffs between packetdrill scripts on github (and Google internal, we have more) and selftests to a minimum. This is invertible, as is rewriting source statements inside the pkt files. But that might be more work and more maintenance in the end.
Thanks again for the pointer and suggestion.
Changing kselftests to preserve directories turns out to be trivial. Patch inline below.
But, existing TARGETS of course then start failing. Because they depend on existing rsync without -R. In (at least) two ways:
amd-pstate fails because its TEST_FILES has includes from other directories and it expects those files to land in the directory with tests.
x86 prefixes all its output with $(OUTPUT) to form absolute paths, which also creates absolute paths in kselftest-list.txt.
These two are examples, not necessarily the one instances of those patterns. So switching to preserving directories for existing targets like TEST_FILES seems intractable.
Plan B is to add a new TEST_PROGS_RECURSE, analogous to how TEST_INCLUDES extended TEST_FILES with optional path preservation. That is not much more complex.
---
+++ b/tools/testing/selftests/kselftest/runner.sh @@ -101,7 +112,7 @@ run_one() echo "# timeout set to $kselftest_timeout" >> "$logfile" fi
- TEST_HDR_MSG="selftests: $DIR: $BASENAME_TEST" + TEST_HDR_MSG="selftests: $DIR: $TEST"
+++ b/tools/testing/selftests/lib.mk @@ -150,7 +150,7 @@ clean_mods_dir:
define INSTALL_SINGLE_RULE $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) - $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/) + $(if $(INSTALL_LIST),rsync -aR --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/) endef
define INSTALL_MODS_RULE @@ -180,8 +180,7 @@ endif
emit_tests: for TEST in $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS); do \ - BASENAME_TEST=`basename $$TEST`; \ - echo "$(COLLECTION):$$BASENAME_TEST"; \ + echo "$(COLLECTION):$$TEST"; \ done