On Thu, Feb 29, 2024 at 4:53 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 Reviewed-by: Sergio Gonzalez Collado sergio.collado@gmail.com Reviewed-by: Muhammad Usama Anjum usama.anjum@collabora.com
Thanks for this Laura!
Replying here to what you wrote in v4:
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.
I guess it depends on what is the expected behavior in kselftests in general and whether the user is expected to have merged the provided `config` or not.
Also, what about modules being built-in / `--first-run` in `modprobe`? `modprobe` by default may return successfully even if no module was loaded (or even present, if it was builtin). In that case, is a kselftest script supposed to succeed, skip or fail? I would say at the least it should be "skip" (like it is done in the case where the module is not found), and I wouldn't mind "fail" either (i.e. running `modprobe` with `--first-run`).
In addition, what about module removal failures? Are they ignored on purpose, e.g. because the kernel might not be configured with module unloading? If it is possible to check whether `MODULE_UNLOAD` is supported in the current config, it would be nice to check the removal also worked. And if it is not supported, skipping the removal entirely.
Finally, what about the case where `RUST` isn't enabled? I think Shuah mentioned it in a previous version.
+KTAP_HELPERS="${DIR}/../kselftest/ktap_helpers.sh" +if [ -e "$KTAP_HELPERS" ]; then
- source "$KTAP_HELPERS"
+else
- echo "$KTAP_HELPERS file not found [SKIP]"
- exit 4
+fi
I am not sure I understand this. In which situation could this happen? The helpers should always be there, no? I tested this with `make -C...../selftests install TARGETS=rust INSTALL_PATH=...` and it seems to work in that case too.
To be clear, I agree with Shuah that we should test that everything is working as expected. In fact, I would prefer to run with `-e` or, much better, use something else than bash :) But if something should never happen, should it be a skip? Shouldn't we just fail because the test infrastructure is somehow missing?
Orthogonally, if we want the test, shouldn't this just test the `source` command directly rather than a proxy (file existing)?
Thanks!
Cheers, Miguel