Received-SPF: pass (google.com: domain of linux-kernel+bounces-14479-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Laura Nao <laura.nao@collabora.com>
To: miguel.ojeda.sandonis@gmail.com
Cc: a.hindborg@samsung.com,
	alex.gaynor@gmail.com,
	aliceryhl@google.com,
	benno.lossin@proton.me,
	bjorn3_gh@protonmail.com,
	boqun.feng@gmail.com,
	davidgow@google.com,
	gary@garyguo.net,
	kernel@collabora.com,
	laura.nao@collabora.com,
	linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	ojeda@kernel.org,
	rust-for-linux@vger.kernel.org,
	shuah@kernel.org,
	wedsonaf@gmail.com
Subject: Re: [PATCH] kselftest: Add basic test for probing the rust sample modules
Date: Tue,  2 Jan 2024 15:25:14 +0100
Message-Id: <20240102142514.174562-1-laura.nao@collabora.com>
In-Reply-To: <CANiq72nAZ53fjsaMk8iYTwtqbvGo3LrTCpBCrh4zx6xLOFBP9w@mail.gmail.com>
References: <CANiq72nAZ53fjsaMk8iYTwtqbvGo3LrTCpBCrh4zx6xLOFBP9w@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 12/21/23 20:46, Miguel Ojeda wrote:
> On Fri, Dec 15, 2023 at 2:21 PM Laura Nao <laura.nao@collabora.com>
> 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@collabora.com>
> 
> 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@collabora.com/T/#u

If that patch is merged first, I'll rework this one and send a v2 with
the proper adjustments.

Best,

Laura