Subject: Re: [PATCH 2/2] selftests/powerpc: Skip test instead of failing

From: Thiago Jung Bauermann <[email-masked]>
Date: Mon, 29 Oct 2018 19:08:44 -0300

Breno Leitao <[email-masked]> writes:

> Hi Tyrel,
>
> On 10/23/2018 05:41 PM, Tyrel Datwyler wrote:
>>> --- a/tools/testing/selftests/powerpc/ptrace/core-pkey.c
>>> +++ b/tools/testing/selftests/powerpc/ptrace/core-pkey.c
>>> @@ -352,17 +352,11 @@ static int write_core_pattern(const char *core_pattern)
>>>  	FILE *f;
>>>
>>>  	f = fopen(core_pattern_file, "w");
>>> -	if (!f) {
>>> -		perror("Error writing to core_pattern file");
>>> -		return TEST_FAIL;
>>> -	}
>>> +	SKIP_IF(!f);
>>>
>>>  	ret = fwrite(core_pattern, 1, len, f);
>>>  	fclose(f);
>>> -	if (ret != len) {
>>> -		perror("Error writing to core_pattern file");
>>> -		return TEST_FAIL;
>>> -	}
>>> +	SKIP_IF(ret != len);
>
>> If we don't have proper privileges we should fail on the open, right?
>> So wouldn't we still want to fail on the write if something goes
>> wrong?
>
> That is a good point. Should the test fail or skip if it is not possible
> to create the infrastructure to run the core test?
>
> Trying to find the answer in the current test sets, I find tests where
> the self test skips if the test environment is not able to be set up, as
> for example, when a memory allocation fails.
>
> File: tools/testing/selftests/powerpc/alignment/alignment_handler.c
>
>         ci1 = mmap(NULL, bufsize, PROT_WRITE, MAP_SHARED,
>                    fd, bufsize);
>         if ((ci0 == MAP_FAILED) || (ci1 == MAP_FAILED)) {
>                 printf("\n");
>                 perror("mmap failed");
>                 SKIP_IF(1);
>         }

I think TEST_FAIL means the test was able to exercise the feature
and found a problem with it. In this case, the test wasn't able to
exercise the feature so it's not appropriate.

Ideally, there should be a TEST_ERROR result for a case like this where
an unexpected problem prevented the testcase from exercising the
feature.

If we're to use the an existing result then I vote for SKIP_IF.

For reference, here are the test results that DejaGnu supports (it is
the test harness used by some GNU projects):

https://www.gnu.org/software/dejagnu/manual/Output-States.html

I would say that SKIP_IF corresponds to UNSUPPORTED in DejaGnu.

--
Thiago Jung Bauermann
IBM Linux Technology Center