Willem de Bruijn wrote:

Jakub Kicinski wrote:

quoted

On Fri, 30 Aug 2024 14:47:43 -0400 Willem de Bruijn wrote:

quoted

quoted

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.

quoted

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