diff mbox series

selftests: openat2: don't print total number of tests and then skip

Message ID 20240522214647.3568788-1-usama.anjum@collabora.com
State New
Headers show
Series selftests: openat2: don't print total number of tests and then skip | expand

Commit Message

Muhammad Usama Anjum May 22, 2024, 9:46 p.m. UTC
Don't print that 88 sub-tests are going to be executed. But then skip.
The error is printed that executed test was only 1 while 88 should have
run:

Old 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

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/openat2/resolve_test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Aleksa Sarai July 1, 2024, 9:14 a.m. UTC | #1
On 2024-07-01, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
> Adding more people for review
> 
> On 5/23/24 2:46 AM, Muhammad Usama Anjum wrote:
> > Don't print that 88 sub-tests are going to be executed. But then skip.
> > The error is printed that executed test was only 1 while 88 should have
> > run:
> > 
> > Old 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
> > 
> > Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> > ---
> >  tools/testing/selftests/openat2/resolve_test.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/openat2/resolve_test.c b/tools/testing/selftests/openat2/resolve_test.c
> > index bbafad440893c..5472ec478d227 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)
> > +	if (geteuid())

This change isn't necessary, != 0 makes what we're checking clearer.

> >  		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)
> 
> -- 
> BR,
> Muhammad Usama Anjum
Muhammad Usama Anjum July 2, 2024, 7:02 a.m. UTC | #2
On 7/1/24 2:14 PM, Aleksa Sarai wrote:
> On 2024-07-01, Muhammad Usama Anjum <usama.anjum@collabora.com> wrote:
>> Adding more people for review
>>
>> On 5/23/24 2:46 AM, Muhammad Usama Anjum wrote:
>>> Don't print that 88 sub-tests are going to be executed. But then skip.
>>> The error is printed that executed test was only 1 while 88 should have
>>> run:
>>>
>>> Old 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
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>>  tools/testing/selftests/openat2/resolve_test.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/openat2/resolve_test.c b/tools/testing/selftests/openat2/resolve_test.c
>>> index bbafad440893c..5472ec478d227 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)
>>> +	if (geteuid())
> 
> This change isn't necessary, != 0 makes what we're checking clearer.
It is wasteful to write != 0 when you can achieve the same without it.

I can update the patch by removing it. Please can you provide a reviewed-by
tag for remaining patch?

> 
>>>  		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)
>>
>> -- 
>> BR,
>> Muhammad Usama Anjum
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/openat2/resolve_test.c b/tools/testing/selftests/openat2/resolve_test.c
index bbafad440893c..5472ec478d227 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)
+	if (geteuid())
 		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)