Re: [PATCH] kselftest: Add basic test for probing the rust sample modules

[Laura Nao]

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