linux-kernel - Re: [PATCH v2] selftests: openat2: don't print total number of tests and then skip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c1414d9d-61b1-4f92-bc8a-333679362283@linuxfoundation.org>
Date: Wed, 31 Jul 2024 10:57:25 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: Muhammad Usama Anjum <usama.anjum@...labora.com>,
 Shuah Khan <shuah@...nel.org>
Cc: Aleksa Sarai <cyphar@...har.com>, kernel@...labora.com,
 linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
 Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2] selftests: openat2: don't print total number of tests
 and then skip

On 7/31/24 07:39, Muhammad Usama Anjum wrote:
> Don't print that 88 sub-tests are going to be executed, but then skip.
> This is against TAP compliance. Instead check pre-requisites first
> before printing total number of tests.

Does TAP clearly mention this?

> 
> Old non-tap compliant output:
>    TAP version 13
>    1..88
>    ok 2 # SKIP all tests require euid == 0
>    # Planned tests != run tests (88 != 1)
>    # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:1 error:0
> 
> New and correct output:
>    TAP version 13
>    1..0 # SKIP all tests require euid == 0

The problem is that this new output doesn't show how many tests
are in this test suite that could be run.

I am not use if this is better for communicating coverage information
even if meets the TAP compliance.

> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> ---
> Changes since v1:
> - Remove simplifying if condition lines
> - Update the patch message
> ---
>   tools/testing/selftests/openat2/resolve_test.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/openat2/resolve_test.c b/tools/testing/selftests/openat2/resolve_test.c
> index bbafad440893c..85a4c64ee950d 100644
> --- a/tools/testing/selftests/openat2/resolve_test.c
> +++ b/tools/testing/selftests/openat2/resolve_test.c
> @@ -508,12 +508,13 @@ void test_openat2_opath_tests(void)
>   int main(int argc, char **argv)
>   {
>   	ksft_print_header();
> -	ksft_set_plan(NUM_TESTS);
>   
>   	/* NOTE: We should be checking for CAP_SYS_ADMIN here... */
>   	if (geteuid() != 0)
>   		ksft_exit_skip("all tests require euid == 0\n");
>   
> +	ksft_set_plan(NUM_TESTS);
> +
>   	test_openat2_opath_tests();
>   
>   	if (ksft_get_fail_cnt() + ksft_get_error_cnt() > 0)

thanks,
-- Shuah