rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] kselftest: Add basic test for probing the rust sample modules
@ 2024-02-26 10:16 Laura Nao
  2024-02-28  0:01 ` Shuah Khan
  0 siblings, 1 reply; 3+ messages in thread
From: Laura Nao @ 2024-02-26 10:16 UTC (permalink / raw)
  To: ojeda, alex.gaynor, wedsonaf, shuah
  Cc: usama.anjum, a.hindborg, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, kernel, laura.nao, linux-kernel,
	linux-kselftest, rust-for-linux, kernel, Sergio Gonzalez Collado

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>
Reviewed-by: Sergio Gonzalez Collado <sergio.collado@gmail.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Depends on:
- https://lore.kernel.org/all/20240102141528.169947-1-laura.nao@collabora.com/T/#u
- https://lore.kernel.org/all/20240131-ktap-sh-helpers-extend-v1-0-98ffb468712c@collabora.com/
Changes in v4:
- Added config file
Changes in v3:
- Removed useless KSFT_PASS, KSFT_FAIL, KSFT_SKIP constants
- Used ktap_finished to print the results summary and handle the return code
Changes in v2:
- Added missing SPDX line
- Edited test_probe_samples.sh script to use the common KTAP helpers file
---
 MAINTAINERS                                   |  1 +
 tools/testing/selftests/Makefile              |  1 +
 tools/testing/selftests/rust/Makefile         |  4 +++
 tools/testing/selftests/rust/config           |  5 +++
 .../selftests/rust/test_probe_samples.sh      | 34 +++++++++++++++++++
 5 files changed, 45 insertions(+)
 create mode 100644 tools/testing/selftests/rust/Makefile
 create mode 100644 tools/testing/selftests/rust/config
 create mode 100755 tools/testing/selftests/rust/test_probe_samples.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index e1475ca38ff2..94e31dac6d2c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19231,6 +19231,7 @@ F:	Documentation/rust/
 F:	rust/
 F:	samples/rust/
 F:	scripts/*rust*
+F:	tools/testing/selftests/rust/
 K:	\b(?i:rust)\b
 
 RXRPC SOCKETS (AF_RXRPC)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index f7255969b695..e1504833654d 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -80,6 +80,7 @@ TARGETS += riscv
 TARGETS += rlimits
 TARGETS += rseq
 TARGETS += rtc
+TARGETS += rust
 TARGETS += seccomp
 TARGETS += sgx
 TARGETS += sigaltstack
diff --git a/tools/testing/selftests/rust/Makefile b/tools/testing/selftests/rust/Makefile
new file mode 100644
index 000000000000..fce1584d3bc0
--- /dev/null
+++ b/tools/testing/selftests/rust/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_PROGS += test_probe_samples.sh
+
+include ../lib.mk
diff --git a/tools/testing/selftests/rust/config b/tools/testing/selftests/rust/config
new file mode 100644
index 000000000000..b4002acd40bc
--- /dev/null
+++ b/tools/testing/selftests/rust/config
@@ -0,0 +1,5 @@
+CONFIG_RUST=y
+CONFIG_SAMPLES=y
+CONFIG_SAMPLES_RUST=y
+CONFIG_SAMPLE_RUST_MINIMAL=m
+CONFIG_SAMPLE_RUST_PRINT=m
\ No newline at end of file
diff --git a/tools/testing/selftests/rust/test_probe_samples.sh b/tools/testing/selftests/rust/test_probe_samples.sh
new file mode 100755
index 000000000000..389d180f14a5
--- /dev/null
+++ b/tools/testing/selftests/rust/test_probe_samples.sh
@@ -0,0 +1,34 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2023 Collabora Ltd
+#
+# This script tests whether the rust sample modules can
+# be added and removed correctly.
+#
+
+DIR="$(dirname "$(readlink -f "$0")")"
+
+source "${DIR}"/../kselftest/ktap_helpers.sh
+
+rust_sample_modules=("rust_minimal" "rust_print")
+
+ktap_print_header
+
+ktap_set_plan "${#rust_sample_modules[@]}"
+
+for sample in "${rust_sample_modules[@]}"; do
+    if ! /sbin/modprobe -n -q "$sample"; then
+        ktap_test_skip "module $sample is not found in /lib/modules/$(uname -r)"
+        continue
+    fi
+
+    if /sbin/modprobe -q "$sample"; then
+        /sbin/modprobe -q -r "$sample"
+        ktap_test_pass "$sample"
+    else
+        ktap_test_fail "$sample"
+    fi
+done
+
+ktap_finished
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] kselftest: Add basic test for probing the rust sample modules
  2024-02-26 10:16 [PATCH v4] kselftest: Add basic test for probing the rust sample modules Laura Nao
@ 2024-02-28  0:01 ` Shuah Khan
  2024-02-29 15:57   ` Laura Nao
  0 siblings, 1 reply; 3+ messages in thread
From: Shuah Khan @ 2024-02-28  0:01 UTC (permalink / raw)
  To: Laura Nao, ojeda, alex.gaynor, wedsonaf, shuah
  Cc: usama.anjum, a.hindborg, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, kernel, linux-kernel, linux-kselftest,
	rust-for-linux, kernel, Sergio Gonzalez Collado, Shuah Khan

Hi Laura,

On 2/26/24 03:16, Laura Nao 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>
> Reviewed-by: Sergio Gonzalez Collado <sergio.collado@gmail.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Depends on:
> - https://lore.kernel.org/all/20240102141528.169947-1-laura.nao@collabora.com/T/#u
> - https://lore.kernel.org/all/20240131-ktap-sh-helpers-extend-v1-0-98ffb468712c@collabora.com/
> Changes in v4:
> - Added config file
> Changes in v3:
> - Removed useless KSFT_PASS, KSFT_FAIL, KSFT_SKIP constants
> - Used ktap_finished to print the results summary and handle the return code
> Changes in v2:
> - Added missing SPDX line
> - Edited test_probe_samples.sh script to use the common KTAP helpers file
> ---
>   MAINTAINERS                                   |  1 +
>   tools/testing/selftests/Makefile              |  1 +
>   tools/testing/selftests/rust/Makefile         |  4 +++
>   tools/testing/selftests/rust/config           |  5 +++
>   .../selftests/rust/test_probe_samples.sh      | 34 +++++++++++++++++++
>   5 files changed, 45 insertions(+)
>   create mode 100644 tools/testing/selftests/rust/Makefile
>   create mode 100644 tools/testing/selftests/rust/config
>   create mode 100755 tools/testing/selftests/rust/test_probe_samples.sh
> 

I ran test again and I still see the same. I would like to
see the script to handle error conditions.

> diff --git a/MAINTAINERS b/MAINTAINERS
>
> +
> +DIR="$(dirname "$(readlink -f "$0")")"
> +
> +source "${DIR}"/../kselftest/ktap_helpers.sh

It tries to source and keeps going. Why can't we test for
the file to exist and skip gracefully without printing
the following messages.

  ./test_probe_samples.sh: line 12: /linux/linux_6.8/tools/testing/selftests/rust/../kselftest/ktap_helpers.sh: No such file or director
# ./test_probe_samples.sh: line 16: ktap_print_header: command not found
# ./test_probe_samples.sh: line 18: ktap_set_plan: command not found
# ./test_probe_samples.sh: line 22: ktap_test_skip: command not found
# ./test_probe_samples.sh: line 22: ktap_test_skip: command not found
# ./test_probe_samples.sh: line 34: ktap_finished: command not found



not ok 1 selftests: rust: test_probe_samples.sh # exit=127


> +
> +rust_sample_modules=("rust_minimal" "rust_print")
> +
> +ktap_print_header
> +
> +ktap_set_plan "${#rust_sample_modules[@]}"
> +
> +for sample in "${rust_sample_modules[@]}"; do
> +    if ! /sbin/modprobe -n -q "$sample"; then
> +        ktap_test_skip "module $sample is not found in /lib/modules/$(uname -r)"
> +        continue

Why are we continuing here? Isn't this skip condition?

> +    fi
> +
> +    if /sbin/modprobe -q "$sample"; then
> +        /sbin/modprobe -q -r "$sample"
> +        ktap_test_pass "$sample"
> +    else
> +        ktap_test_fail "$sample"
> +    fi
> +done
> +
> +ktap_finished


I would like to see the test exit with skip code when RUST isn't
enabled. Please refer to existing tests that do this properly.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] kselftest: Add basic test for probing the rust sample modules
  2024-02-28  0:01 ` Shuah Khan
@ 2024-02-29 15:57   ` Laura Nao
  0 siblings, 0 replies; 3+ messages in thread
From: Laura Nao @ 2024-02-29 15:57 UTC (permalink / raw)
  To: skhan
  Cc: a.hindborg, alex.gaynor, aliceryhl, benno.lossin, bjorn3_gh,
	boqun.feng, gary, kernel, kernel, laura.nao, linux-kernel,
	linux-kselftest, ojeda, rust-for-linux, sergio.collado, shuah,
	usama.anjum, wedsonaf

Hi Shuah,

On 2/28/24 01:01, Shuah Khan wrote:
> Hi Laura,
> 
> On 2/26/24 03:16, Laura Nao 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>
>> Reviewed-by: Sergio Gonzalez Collado <sergio.collado@gmail.com>
>> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Depends on:
>> - 
>> https://lore.kernel.org/all/20240102141528.169947-1-laura.nao@collabora.com/T/#u
>> - 
>> https://lore.kernel.org/all/20240131-ktap-sh-helpers-extend-v1-0-98ffb468712c@collabora.com/
>> Changes in v4:
>> - Added config file
>> Changes in v3:
>> - Removed useless KSFT_PASS, KSFT_FAIL, KSFT_SKIP constants
>> - Used ktap_finished to print the results summary and handle the 
>> return code
>> Changes in v2:
>> - Added missing SPDX line
>> - Edited test_probe_samples.sh script to use the common KTAP helpers file
>> ---
>>   MAINTAINERS                                   |  1 +
>>   tools/testing/selftests/Makefile              |  1 +
>>   tools/testing/selftests/rust/Makefile         |  4 +++
>>   tools/testing/selftests/rust/config           |  5 +++
>>   .../selftests/rust/test_probe_samples.sh      | 34 +++++++++++++++++++
>>   5 files changed, 45 insertions(+)
>>   create mode 100644 tools/testing/selftests/rust/Makefile
>>   create mode 100644 tools/testing/selftests/rust/config
>>   create mode 100755 tools/testing/selftests/rust/test_probe_samples.sh
>>
> 
> I ran test again and I still see the same. I would like to
> see the script to handle error conditions.
> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>>
>> +
>> +DIR="$(dirname "$(readlink -f "$0")")"
>> +
>> +source "${DIR}"/../kselftest/ktap_helpers.sh
> 
> It tries to source and keeps going. Why can't we test for
> the file to exist and skip gracefully without printing
> the following messages.
> 
>   ./test_probe_samples.sh: line 12: 
> /linux/linux_6.8/tools/testing/selftests/rust/../kselftest/ktap_helpers.sh: No such file or director
> # ./test_probe_samples.sh: line 16: ktap_print_header: command not found
> # ./test_probe_samples.sh: line 18: ktap_set_plan: command not found
> # ./test_probe_samples.sh: line 22: ktap_test_skip: command not found
> # ./test_probe_samples.sh: line 22: ktap_test_skip: command not found
> # ./test_probe_samples.sh: line 34: ktap_finished: command not found
> 
> 
> 
> not ok 1 selftests: rust: test_probe_samples.sh # exit=127
> 
> 

Sorry, I misunderstood the request in your previous reply on v3 - will 
do.

>> +
>> +rust_sample_modules=("rust_minimal" "rust_print")
>> +
>> +ktap_print_header
>> +
>> +ktap_set_plan "${#rust_sample_modules[@]}"
>> +
>> +for sample in "${rust_sample_modules[@]}"; do
>> +    if ! /sbin/modprobe -n -q "$sample"; then
>> +        ktap_test_skip "module $sample is not found in 
>> /lib/modules/$(uname -r)"
>> +        continue
> 
> Why are we continuing here? Isn't this skip condition?
>

At first, I hadn't planned for the kselftest to skip entirely if only
one of the two sample modules was missing. However, considering that 
this kselftest is designed to test all available sample modules, and 
given that both are enabled with the provided configuration file, I 
believe it's more logical to verify the presence of both modules before 
running the test. If either of them is missing, then we exit the test 
with a skip code. This also covers the case where rust is not available.

>> +    fi
>> +
>> +    if /sbin/modprobe -q "$sample"; then
>> +        /sbin/modprobe -q -r "$sample"
>> +        ktap_test_pass "$sample"
>> +    else
>> +        ktap_test_fail "$sample"
>> +    fi
>> +done
>> +
>> +ktap_finished
> 
> 
> I would like to see the test exit with skip code when RUST isn't
> enabled. Please refer to existing tests that do this properly.
> 

Sent a v5 with the changes mentioned above: https://lore.kernel.org/linux-kselftest/20240229155235.263157-1-laura.nao@collabora.com/T/#u

Thanks!

Laura

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-02-29 15:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-26 10:16 [PATCH v4] kselftest: Add basic test for probing the rust sample modules Laura Nao
2024-02-28  0:01 ` Shuah Khan
2024-02-29 15:57   ` Laura Nao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).