diff mbox series

[v3,1/4] selftests: x86: check_initial_reg_state: remove manual counting and increase maintainability

Message ID 20240718113222.867116-2-usama.anjum@collabora.com (mailing list archive)
State New
Headers show
Series selftest: x86: improve tests | expand

Commit Message

Muhammad Usama Anjum July 18, 2024, 11:32 a.m. UTC
Removes manual counting of pass and fail tests. This increases readability
of tests, but also improves maintainability of the tests. Print logs in
standard format (without [RUN], [OK] tags)

Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
Changes since v1:
- correct description of the patch

Changes since v2:
- Update description of the patch and add before/after output

Before:
  # selftests: x86: check_initial_reg_state_32
  # [OK]	All GPRs except SP are 0
  # [OK]	FLAGS is 0x202
  ok 5 selftests: x86: check_initial_reg_state_32

After:
  # selftests: x86: check_initial_reg_state_32
  # TAP version 13
  # 1..2
  # ok 1 All GPRs except SP are 0
  # ok 2 FLAGS is 0x202
  # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
  ok 5 selftests: x86: check_initial_reg_state_32
---
 .../selftests/x86/check_initial_reg_state.c   | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

Comments

Shuah Khan July 18, 2024, 3:48 p.m. UTC | #1
On 7/18/24 05:32, Muhammad Usama Anjum wrote:
> Removes manual counting of pass and fail tests. This increases readability
> of tests, but also improves maintainability of the tests. Print logs in
> standard format (without [RUN], [OK] tags)
> 
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
> Changes since v1:
> - correct description of the patch
> 
> Changes since v2:
> - Update description of the patch and add before/after output
> 
> Before:
>    # selftests: x86: check_initial_reg_state_32
>    # [OK]	All GPRs except SP are 0
>    # [OK]	FLAGS is 0x202
>    ok 5 selftests: x86: check_initial_reg_state_32
> 
> After:
>    # selftests: x86: check_initial_reg_state_32
>    # TAP version 13
>    # 1..2
>    # ok 1 All GPRs except SP are 0
>    # ok 2 FLAGS is 0x202
>    # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>    ok 5 selftests: x86: check_initial_reg_state_32

What's the output you see if you were run this as:

make ksefltest TARGETS=x86

How is this different from the output from the above command?

Please provide the same information for your other patches in this
series

thanks,
-- Shuah
Muhammad Usama Anjum July 19, 2024, 7:28 a.m. UTC | #2
On 7/18/24 8:48 PM, Shuah Khan wrote:
> On 7/18/24 05:32, Muhammad Usama Anjum wrote:
>> Removes manual counting of pass and fail tests. This increases readability
>> of tests, but also improves maintainability of the tests. Print logs in
>> standard format (without [RUN], [OK] tags)
>>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>> Changes since v1:
>> - correct description of the patch
>>
>> Changes since v2:
>> - Update description of the patch and add before/after output
>>
>> Before:
>>    # selftests: x86: check_initial_reg_state_32
>>    # [OK]    All GPRs except SP are 0
>>    # [OK]    FLAGS is 0x202
>>    ok 5 selftests: x86: check_initial_reg_state_32
>>
>> After:
>>    # selftests: x86: check_initial_reg_state_32
>>    # TAP version 13
>>    # 1..2
>>    # ok 1 All GPRs except SP are 0
>>    # ok 2 FLAGS is 0x202
>>    # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>>    ok 5 selftests: x86: check_initial_reg_state_32
> 
> What's the output you see if you were run this as:
> 
> make ksefltest TARGETS=x86
> 
> How is this different from the output from the above command?
The above before and after output has been taken by executing this above
command. I've copy/pasted the snippets for this patch only.

> 
> Please provide the same information for your other patches in this
> series
All other patches have this information already.


> 
> thanks,
> -- Shuah
>
Shuah Khan July 19, 2024, 4:52 p.m. UTC | #3
On 7/19/24 01:28, Muhammad Usama Anjum wrote:
> On 7/18/24 8:48 PM, Shuah Khan wrote:
>> On 7/18/24 05:32, Muhammad Usama Anjum wrote:
>>> Removes manual counting of pass and fail tests. This increases readability
>>> of tests, but also improves maintainability of the tests. Print logs in
>>> standard format (without [RUN], [OK] tags)
>>>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>> Changes since v1:
>>> - correct description of the patch
>>>
>>> Changes since v2:
>>> - Update description of the patch and add before/after output
>>>
>>> Before:
>>>     # selftests: x86: check_initial_reg_state_32
>>>     # [OK]    All GPRs except SP are 0
>>>     # [OK]    FLAGS is 0x202
>>>     ok 5 selftests: x86: check_initial_reg_state_32
>>>
>>> After:
>>>     # selftests: x86: check_initial_reg_state_32
>>>     # TAP version 13
>>>     # 1..2
>>>     # ok 1 All GPRs except SP are 0
>>>     # ok 2 FLAGS is 0x202
>>>     # # Totals: pass:2 fail:0 xfail:0 xpass:0 skip:0 error:0
>>>     ok 5 selftests: x86: check_initial_reg_state_32
>>
>> What's the output you see if you were run this as:
>>
>> make ksefltest TARGETS=x86
>>

That is what is confusing to me. As mentioned in response to your
vDSO patch, this change to add ksft_header to individual tests.

When you run the test from the wrapper
if you want the header printed for q

>> How is this different from the output from the above command?
> The above before and after output has been taken by executing this above
> command. I've copy/pasted the snippets for this patch only.
> 

Yes. That is the problem. Youa re giving me snippets as opposed to
the header. When I run it I see TAP header at the top of the test
suite. The idea is that the TAP header should not be printed for
each test in the test suite.

It is printed once for test suite. What is the point in printing TAP
header for each of the tests in test suite if there are 100 tests
like in the case of break_points test.

Even if it is desired that each individual should have TAP header
(There has to be a good reason why - not because we want to see it),
this change should be added to
   
tools/testing/selftests/kselftest/runner.sh
This makes it uniform and maintainable.

Sorry. I am not going accept patches that add ksft header to individual
tests and test cases.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/tools/testing/selftests/x86/check_initial_reg_state.c b/tools/testing/selftests/x86/check_initial_reg_state.c
index 3bc95f3ed5859..b91c2b06b9881 100644
--- a/tools/testing/selftests/x86/check_initial_reg_state.c
+++ b/tools/testing/selftests/x86/check_initial_reg_state.c
@@ -7,6 +7,7 @@ 
 #define _GNU_SOURCE
 
 #include <stdio.h>
+#include "../kselftest.h"
 
 unsigned long ax, bx, cx, dx, si, di, bp, sp, flags;
 unsigned long r8, r9, r10, r11, r12, r13, r14, r15;
@@ -53,20 +54,19 @@  asm (
 
 int main()
 {
-	int nerrs = 0;
+	ksft_print_header();
+	ksft_set_plan(2);
 
-	if (sp == 0) {
-		printf("[FAIL]\tTest was built incorrectly\n");
-		return 1;
-	}
+	if (sp == 0)
+		ksft_exit_fail_msg("Test was built incorrectly\n");
 
 	if (ax || bx || cx || dx || si || di || bp
 #ifdef __x86_64__
 	    || r8 || r9 || r10 || r11 || r12 || r13 || r14 || r15
 #endif
 		) {
-		printf("[FAIL]\tAll GPRs except SP should be 0\n");
-#define SHOW(x) printf("\t" #x " = 0x%lx\n", x);
+		ksft_test_result_fail("All GPRs except SP should be 0\n");
+#define SHOW(x) ksft_print_msg("\t" #x " = 0x%lx\n", x)
 		SHOW(ax);
 		SHOW(bx);
 		SHOW(cx);
@@ -85,17 +85,15 @@  int main()
 		SHOW(r14);
 		SHOW(r15);
 #endif
-		nerrs++;
 	} else {
-		printf("[OK]\tAll GPRs except SP are 0\n");
+		ksft_test_result_pass("All GPRs except SP are 0\n");
 	}
 
 	if (flags != 0x202) {
-		printf("[FAIL]\tFLAGS is 0x%lx, but it should be 0x202\n", flags);
-		nerrs++;
+		ksft_test_result_fail("FLAGS is 0x%lx, but it should be 0x202\n", flags);
 	} else {
-		printf("[OK]\tFLAGS is 0x202\n");
+		ksft_test_result_pass("FLAGS is 0x202\n");
 	}
 
-	return nerrs ? 1 : 0;
+	ksft_finished();
 }