Re: hmm_test issues with latest mainline

[Alistair Popple <apopple@nvidia.com>]

Seems like this would fix both the SKIP in FIXTURE_SETUP and ASSERT in
FIXTURE_TEARDOWN issues:

---

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 25f4d54067c0..1998fe888f8f 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -387,12 +387,12 @@
 		if (setjmp(_metadata->env) == 0) { \
 			fixture_name##_setup(_metadata, &self, variant->data); \
 			/* Let setup failure terminate early. */ \
-			if (!_metadata->passed) \
+			if (!_metadata->passed || _metadata->skip) \
 				return; \
 			_metadata->setup_completed = true; \
 			fixture_name##_##test_name(_metadata, &self, variant->data); \
 		} \
-		if (_metadata->setup_completed) \
+		if (_metadata->setup_completed && setjmp(_metadata->env) == 0) \
 			fixture_name##_teardown(_metadata, &self, variant->data); \
 		__test_check_assert(_metadata); \
 	} \

Alistair Popple <apopple@nvidia.com> writes:

> Vlastimil Babka <vbabka@suse.cz> writes:
>
>> On 10/13/2022 9:38 PM, Shuah Khan wrote:
>>> On 10/13/22 12:00, David Hildenbrand wrote:
>>>>>>>> When did that test start failing? Was it still ok for 6.0?
>>>>>>
>>>>>> Didn't test yet, will try, in case it's my system/config specific thing.
>>>>>
>>>>> So it's actually all the same with v6.0 for me. The infinite loops, the test
>>>>> failures, the misreported SKIPs.
>>>>>
>>>
>>> I am not seeing infinite loops and seeing 25 failures which could
>>> be skips.
>>>
>>>>
>>>> Is the kernel compiled with support. I have the feeling that we might simply miss kernel support and it's not handled gracefully ...
>>>>
>>>
>>> Here is my config
>>> CONFIG_HMM_MIRROR=y
>>> # CONFIG_TEST_HMM is not set
>>>
>>> Okay here is what is going on - hmm_tests are supposed to be run
>>> from test_hmm.sh script. When I run this I see a message that tells
>>> me what to do.
>>>
>>> sudo ./test_hmm.sh
>>> ./test_hmm.sh: You must have the following enabled in your kernel:
>>> CONFIG_TEST_HMM=m
>>>
>>> Running ./hmm_tests gives me all the failures. So it appears running
>>> hmm_tests executable won't work. This is expected as test_hmm.sh does
>>> the right setup before running the test. We have several tests that do
>>> that.
>>>
>>> Vlastimil, can you try this and let me know what you see. I will compile
>>> with CONFIG_TEST_HMM=m and let you know what I see on my system.
>>
>> Right, I didn't mention it, sorry. I did have CONFIG_TEST_HMM=m and was running
>> "test_hmm.sh smoke"
>
> FWIW I tend not to use that script on my development machine, mainly
> because I either have the module built in or otherwise don't have
> modules installed in a place modprobe knows about.
>
> Anyway I am not seeing test failures running hmm-tests directly. However
> I do observe both the issue of SKIP in FIXTURE_SETUP() being reported as
> a pass in the summary, and the infinite loop on ASSERT failure in
> FIXTURE_TEARDOWN.
>
> There does seem to be some framework issues here which are causing this
> behaviour. Consider the following representitive snippet:
>
> #include "../kselftest_harness.h"
>
> #include <stdio.h>
>
> FIXTURE(test) {};
>
> FIXTURE_SETUP(test)
> {
> 	SKIP(return, "skip");
> }
>
> FIXTURE_TEARDOWN(test)
> {
> 	ASSERT_TRUE(0);
> }
>
> TEST_F(test, test)
> {
> 	printf("Running test\n");
> }
>
> TEST_HARNESS_MAIN
>
> In this case the test will still be run even though SKIP() was called in
> FIXTURE_SETUP. The ASSERT_TRUE() during FIXTURE_TEARDOWN results in the
> infinite loop. So it looks to me like calling SKIP from FIXTURE_SETUP
> isn't supported, and calling ASSERT_*() in FIXTURE_TEARDOWN is also not
> allowed/supported by the kselftest framework.
>
> Unlike hmm-tests though the above snippet reports correct pass/skip
> statistics with the teardown assertion removed. This is because there is
> also a bug in hmm-tests. Currently we have:
>
>    SKIP(exit(0), "DEVICE_COHERENT not available");
>
> Which should really be:
>
>    SKIP(return, "DEVICE_COHERENT not available");
>
> Of course that results in an infinite loop due to the associated
> assertion failure during teardown which is still called despite the SKIP
> in setup. Not sure if this is why it was originally coded this way.
>
>  - Alistair
>
>>> thanks,
>>> -- Shuah
>>>
>>>
>>>