On 12/21/23 20:46, Miguel Ojeda wrote: > On Fri, Dec 15, 2023 at 2:21 PM Laura Nao <laura.nao@xxxxxxxxxxxxx> > wrote: >> >> Add new basic kselftest that checks if the available rust sample >> modules >> can be added and removed correctly. >> >> Signed-off-by: Laura Nao <laura.nao@xxxxxxxxxxxxx> > > Thanks Laura! > > Shuah: do you want that we pick this one? If so, your `Acked-by` would > be nice -- thanks! Otherwise, please feel free to pick it up. > > Cc'ing David too since it involves KTAP in case he has comments. > >> diff --git a/tools/testing/selftests/rust/Makefile >> b/tools/testing/selftests/rust/Makefile > > Missing SPDX line? (it can be added when picking it up, though). > Thanks for the feedback Miguel! >> +$(OUTPUT)/ktap_helpers.sh: >> + cp $(top_srcdir)/tools/testing/selftests/dt/ktap_helpers.sh >> $@ > > This may be something for another series, but should these helpers be > factored out perhaps / provided by the framework? Does it work > sourcing them from `dt` directly instead of copying meanwhile (to > simplify)? > >> +KSFT_PASS=0 >> +KSFT_FAIL=1 >> +KSFT_SKIP=4 > > Similarly, would it make sense for this kind of "common constants" be > factored somehow? Or does that not make sense (I see other tests also > define them "manually")? > Sourcing the file from the `dt` folder does work when running the test with `make -C tools/testing/selftests TARGETS=rust run_tests`, but fails when the test is installed with `make -C tools/testing/selftests TARGETS=rust install` and run with `./tools/testing/selftests/kselftest_install/run_kselftest.sh` (unless the dt kselftest is installed too). I agree factoring out the helpers might be a better solution. I sent a patch to move the ktap_helpers.sh file to the kselftest common directory, so that kselftests written in bash can make use of the helper functions more easily: https://lore.kernel.org/linux-kselftest/20240102141528.169947-1-laura.nao@xxxxxxxxxxxxx/T/#u If that patch is merged first, I'll rework this one and send a v2 with the proper adjustments. Best, Laura
