[PATCH 0/3] Add kselftest harness selftest with variant

[PATCH 0/3] Add kselftest harness selftest with variant

[Wei Yang]

We already have a selftest for harness, while there is not usage of
FIXTURE_VARIANT.

Patch 3 add FIXTURE_VARIANT usage in the selftest.

Patch 1/2 are trivial fix.

Wei Yang (3):
  selftests: correct one typo in comment
  selftests: print 0 if no test is chosen
  selftests: harness: Add kselftest harness selftest with variant

 tools/testing/selftests/kselftest.h           |  2 +-
 tools/testing/selftests/kselftest_harness.h   |  2 +-
 .../kselftest_harness/harness-selftest.c      | 34 +++++++++++++++++++
 .../harness-selftest.expected                 | 22 +++++++++---
 4 files changed, 54 insertions(+), 6 deletions(-)

-- 
2.34.1

[PATCH 1/3] selftests: correct one typo in comment

[Wei Yang]

The name is __constructor_order_forward.

Just correct it.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 tools/testing/selftests/kselftest_harness.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 2925e47db995..674a6112e6e1 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -936,7 +936,7 @@ static inline bool __test_passed(struct __test_metadata *metadata)
  * list so tests are run in source declaration order.
  * https://gcc.gnu.org/onlinedocs/gccint/Initialization.html
  * However, it seems not all toolchains do this correctly, so use
- * __constructor_order_foward to detect which direction is called first
+ * __constructor_order_forward to detect which direction is called first
  * and adjust list building logic to get things running in the right
  * direction.
  */
-- 
2.34.1

Re: [PATCH 1/3] selftests: correct one typo in comment

[Thomas Weißschuh]

Please use "selftests: harness:" as subject prefix for the patches.
Also mention the specific typo to make the subject more unique.

On Mon, Jun 16, 2025 at 12:23:36PM +0000, Wei Yang wrote:
> The name is __constructor_order_forward.
>
> Just correct it.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

With the above comments addressed:
Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

> ---
>  tools/testing/selftests/kselftest_harness.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 2925e47db995..674a6112e6e1 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -936,7 +936,7 @@ static inline bool __test_passed(struct __test_metadata *metadata)
>   * list so tests are run in source declaration order.
>   * https://gcc.gnu.org/onlinedocs/gccint/Initialization.html
>   * However, it seems not all toolchains do this correctly, so use
> - * __constructor_order_foward to detect which direction is called first
> + * __constructor_order_forward to detect which direction is called first
>   * and adjust list building logic to get things running in the right
>   * direction.
>   */
> -- 
> 2.34.1
> 

Re: [PATCH 1/3] selftests: correct one typo in comment

[Wei Yang]

On Tue, Jun 17, 2025 at 09:31:52AM +0200, Thomas Weißschuh wrote:
>Please use "selftests: harness:" as subject prefix for the patches.
>Also mention the specific typo to make the subject more unique.
>

Thanks.

Will adjust it.

>On Mon, Jun 16, 2025 at 12:23:36PM +0000, Wei Yang wrote:
>> The name is __constructor_order_forward.
>>
>> Just correct it.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>
>With the above comments addressed:
>Reviewed-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>
>> ---
>>  tools/testing/selftests/kselftest_harness.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
>> index 2925e47db995..674a6112e6e1 100644
>> --- a/tools/testing/selftests/kselftest_harness.h
>> +++ b/tools/testing/selftests/kselftest_harness.h
>> @@ -936,7 +936,7 @@ static inline bool __test_passed(struct __test_metadata *metadata)
>>   * list so tests are run in source declaration order.
>>   * https://gcc.gnu.org/onlinedocs/gccint/Initialization.html
>>   * However, it seems not all toolchains do this correctly, so use
>> - * __constructor_order_foward to detect which direction is called first
>> + * __constructor_order_forward to detect which direction is called first
>>   * and adjust list building logic to get things running in the right
>>   * direction.
>>   */
>> -- 
>> 2.34.1
>> 

-- 
Wei Yang
Help you, Help me

[PATCH 2/3] selftests: print 0 if no test is chosen

[Wei Yang]

In case there is no test chosen, e.g -t non-exist, the following message
would be printed at start.

    TAP version 13
    1..0

Change it to print 0 if no test is chosen.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 tools/testing/selftests/kselftest.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
index c3b6d2604b1e..9fcf76f0b702 100644
--- a/tools/testing/selftests/kselftest.h
+++ b/tools/testing/selftests/kselftest.h
@@ -144,7 +144,7 @@ static inline void ksft_print_header(void)
 static inline void ksft_set_plan(unsigned int plan)
 {
 	ksft_plan = plan;
-	printf("1..%u\n", ksft_plan);
+	printf("%u..%u\n", !plan ? 0 : 1, ksft_plan);
 }
 
 static inline void ksft_print_cnts(void)
-- 
2.34.1

Re: [PATCH 2/3] selftests: print 0 if no test is chosen

[Muhammad Usama Anjum]

On 6/16/25 5:23 PM, Wei Yang wrote:
> In case there is no test chosen, e.g -t non-exist, the following message
> would be printed at start.
> 
>     TAP version 13
>     1..0
> 
> Change it to print 0 if no test is chosen.
Please give reference from TAP format guidelines for this change.

> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  tools/testing/selftests/kselftest.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index c3b6d2604b1e..9fcf76f0b702 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -144,7 +144,7 @@ static inline void ksft_print_header(void)
>  static inline void ksft_set_plan(unsigned int plan)
>  {
>  	ksft_plan = plan;
> -	printf("1..%u\n", ksft_plan);
> +	printf("%u..%u\n", !plan ? 0 : 1, ksft_plan);
>  }
>  
>  static inline void ksft_print_cnts(void)

Re: [PATCH 2/3] selftests: print 0 if no test is chosen

[Wei Yang]

Hi, Muhammad

Thanks for review.

On Mon, Jun 16, 2025 at 05:44:55PM +0500, Muhammad Usama Anjum wrote:
>On 6/16/25 5:23 PM, Wei Yang wrote:
>> In case there is no test chosen, e.g -t non-exist, the following message
>> would be printed at start.
>> 
>>     TAP version 13
>>     1..0
>> 
>> Change it to print 0 if no test is chosen.
>Please give reference from TAP format guidelines for this change.

I see, will drop it for later version. 

>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>  tools/testing/selftests/kselftest.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
>> index c3b6d2604b1e..9fcf76f0b702 100644
>> --- a/tools/testing/selftests/kselftest.h
>> +++ b/tools/testing/selftests/kselftest.h
>> @@ -144,7 +144,7 @@ static inline void ksft_print_header(void)
>>  static inline void ksft_set_plan(unsigned int plan)
>>  {
>>  	ksft_plan = plan;
>> -	printf("1..%u\n", ksft_plan);
>> +	printf("%u..%u\n", !plan ? 0 : 1, ksft_plan);
>>  }
>>  
>>  static inline void ksft_print_cnts(void)

-- 
Wei Yang
Help you, Help me

[PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Wei Yang]

Each fixture could support variant. Add fixture with variant to verify
the behavior, so we can validate for further change.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
---
 .../kselftest_harness/harness-selftest.c      | 34 +++++++++++++++++++
 .../harness-selftest.expected                 | 22 +++++++++---
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.c b/tools/testing/selftests/kselftest_harness/harness-selftest.c
index b555493bdb4d..2fd5310b33c7 100644
--- a/tools/testing/selftests/kselftest_harness/harness-selftest.c
+++ b/tools/testing/selftests/kselftest_harness/harness-selftest.c
@@ -118,6 +118,40 @@ TEST_F(fixture_setup_failure, pass) {
 	TH_LOG("after");
 }
 
+FIXTURE(fixture_variant) {
+	pid_t testpid;
+};
+
+FIXTURE_VARIANT(fixture_variant)
+{
+	int value;
+};
+
+FIXTURE_VARIANT_ADD(fixture_variant, v32)
+{
+	.value = 32,
+};
+
+FIXTURE_VARIANT_ADD(fixture_variant, v64)
+{
+	.value = 64,
+};
+
+FIXTURE_SETUP(fixture_variant) {
+	TH_LOG("setup %d", variant->value);
+	self->testpid = getpid();
+}
+
+FIXTURE_TEARDOWN(fixture_variant) {
+	TH_LOG("teardown same-process=%d", self->testpid == getpid());
+}
+
+TEST_F(fixture_variant, pass) {
+	TH_LOG("before");
+	ASSERT_EQ(0, 0);
+	TH_LOG("after");
+}
+
 int main(int argc, char **argv)
 {
 	/*
diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.expected b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
index 97e1418c1c7e..ab081c5aba05 100644
--- a/tools/testing/selftests/kselftest_harness/harness-selftest.expected
+++ b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
@@ -1,6 +1,6 @@
 TAP version 13
-1..9
-# Starting 9 tests from 4 test cases.
+1..11
+# Starting 11 tests from 6 test cases.
 #  RUN           global.standalone_pass ...
 # harness-selftest.c:19:standalone_pass:before
 # harness-selftest.c:23:standalone_pass:after
@@ -60,5 +60,19 @@ ok 8 fixture_parent.pass
 # pass: Test terminated by assertion
 #          FAIL  fixture_setup_failure.pass
 not ok 9 fixture_setup_failure.pass
-# FAILED: 4 / 9 tests passed.
-# Totals: pass:4 fail:5 xfail:0 xpass:0 skip:0 error:0
+#  RUN           fixture_variant.v32.pass ...
+# harness-selftest.c:141:pass:setup 32
+# harness-selftest.c:150:pass:before
+# harness-selftest.c:152:pass:after
+# harness-selftest.c:146:pass:teardown same-process=1
+#            OK  fixture_variant.v32.pass
+ok 10 fixture_variant.v32.pass
+#  RUN           fixture_variant.v64.pass ...
+# harness-selftest.c:141:pass:setup 64
+# harness-selftest.c:150:pass:before
+# harness-selftest.c:152:pass:after
+# harness-selftest.c:146:pass:teardown same-process=1
+#            OK  fixture_variant.v64.pass
+ok 11 fixture_variant.v64.pass
+# FAILED: 6 / 11 tests passed.
+# Totals: pass:6 fail:5 xfail:0 xpass:0 skip:0 error:0
-- 
2.34.1

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Muhammad Usama Anjum]

On 6/16/25 5:23 PM, Wei Yang wrote:
> Each fixture could support variant. Add fixture with variant to verify
> the behavior, so we can validate for further change.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>

> ---
>  .../kselftest_harness/harness-selftest.c      | 34 +++++++++++++++++++
>  .../harness-selftest.expected                 | 22 +++++++++---
>  2 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.c b/tools/testing/selftests/kselftest_harness/harness-selftest.c
> index b555493bdb4d..2fd5310b33c7 100644
> --- a/tools/testing/selftests/kselftest_harness/harness-selftest.c
> +++ b/tools/testing/selftests/kselftest_harness/harness-selftest.c
> @@ -118,6 +118,40 @@ TEST_F(fixture_setup_failure, pass) {
>  	TH_LOG("after");
>  }
>  
> +FIXTURE(fixture_variant) {
> +	pid_t testpid;
> +};
> +
> +FIXTURE_VARIANT(fixture_variant)
> +{
> +	int value;
> +};
> +
> +FIXTURE_VARIANT_ADD(fixture_variant, v32)
> +{
> +	.value = 32,
> +};
> +
> +FIXTURE_VARIANT_ADD(fixture_variant, v64)
> +{
> +	.value = 64,
> +};
> +
> +FIXTURE_SETUP(fixture_variant) {
> +	TH_LOG("setup %d", variant->value);
> +	self->testpid = getpid();
> +}
> +
> +FIXTURE_TEARDOWN(fixture_variant) {
> +	TH_LOG("teardown same-process=%d", self->testpid == getpid());
> +}
> +
> +TEST_F(fixture_variant, pass) {
> +	TH_LOG("before");
> +	ASSERT_EQ(0, 0);
> +	TH_LOG("after");
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	/*
> diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.expected b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> index 97e1418c1c7e..ab081c5aba05 100644
> --- a/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> +++ b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> @@ -1,6 +1,6 @@
>  TAP version 13
> -1..9
> -# Starting 9 tests from 4 test cases.
> +1..11
> +# Starting 11 tests from 6 test cases.
>  #  RUN           global.standalone_pass ...
>  # harness-selftest.c:19:standalone_pass:before
>  # harness-selftest.c:23:standalone_pass:after
> @@ -60,5 +60,19 @@ ok 8 fixture_parent.pass
>  # pass: Test terminated by assertion
>  #          FAIL  fixture_setup_failure.pass
>  not ok 9 fixture_setup_failure.pass
> -# FAILED: 4 / 9 tests passed.
> -# Totals: pass:4 fail:5 xfail:0 xpass:0 skip:0 error:0
> +#  RUN           fixture_variant.v32.pass ...
> +# harness-selftest.c:141:pass:setup 32
> +# harness-selftest.c:150:pass:before
> +# harness-selftest.c:152:pass:after
> +# harness-selftest.c:146:pass:teardown same-process=1
> +#            OK  fixture_variant.v32.pass
> +ok 10 fixture_variant.v32.pass
> +#  RUN           fixture_variant.v64.pass ...
> +# harness-selftest.c:141:pass:setup 64
> +# harness-selftest.c:150:pass:before
> +# harness-selftest.c:152:pass:after
> +# harness-selftest.c:146:pass:teardown same-process=1
> +#            OK  fixture_variant.v64.pass
> +ok 11 fixture_variant.v64.pass
> +# FAILED: 6 / 11 tests passed.
> +# Totals: pass:6 fail:5 xfail:0 xpass:0 skip:0 error:0

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Wei Yang]

On Mon, Jun 16, 2025 at 05:45:42PM +0500, Muhammad Usama Anjum wrote:
>On 6/16/25 5:23 PM, Wei Yang wrote:
>> Each fixture could support variant. Add fixture with variant to verify
>> the behavior, so we can validate for further change.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>

Hi, Muhammad

Since I may change the code in v2, I think I should drop your RB.

But will cc you. Hope you would take a look later. Thanks

-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Thomas Weißschuh]

Good idea.

On Mon, Jun 16, 2025 at 12:23:38PM +0000, Wei Yang wrote:
> Each fixture could support variant. Add fixture with variant to verify
> the behavior, so we can validate for further change.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> ---
>  .../kselftest_harness/harness-selftest.c      | 34 +++++++++++++++++++
>  .../harness-selftest.expected                 | 22 +++++++++---
>  2 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.c b/tools/testing/selftests/kselftest_harness/harness-selftest.c
> index b555493bdb4d..2fd5310b33c7 100644
> --- a/tools/testing/selftests/kselftest_harness/harness-selftest.c
> +++ b/tools/testing/selftests/kselftest_harness/harness-selftest.c
> @@ -118,6 +118,40 @@ TEST_F(fixture_setup_failure, pass) {
>  	TH_LOG("after");
>  }
>  
> +FIXTURE(fixture_variant) {
> +	pid_t testpid;
> +};
> +
> +FIXTURE_VARIANT(fixture_variant)
> +{
> +	int value;
> +};
> +
> +FIXTURE_VARIANT_ADD(fixture_variant, v32)
> +{
> +	.value = 32,
> +};
> +
> +FIXTURE_VARIANT_ADD(fixture_variant, v64)
> +{
> +	.value = 64,
> +};
> +
> +FIXTURE_SETUP(fixture_variant) {
> +	TH_LOG("setup %d", variant->value);
> +	self->testpid = getpid();
> +}
> +
> +FIXTURE_TEARDOWN(fixture_variant) {
> +	TH_LOG("teardown same-process=%d", self->testpid == getpid());
> +}
> +
> +TEST_F(fixture_variant, pass) {
> +	TH_LOG("before");
> +	ASSERT_EQ(0, 0);

Please log the variant value from the test itself and the teardown function.
Also I don't think we need the pid logging and before/after/ASSERT in this test
also, it is already validated in the other ones.

> +	TH_LOG("after");
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	/*
> diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.expected b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> index 97e1418c1c7e..ab081c5aba05 100644
> --- a/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> +++ b/tools/testing/selftests/kselftest_harness/harness-selftest.expected
> @@ -1,6 +1,6 @@
>  TAP version 13
> -1..9
> -# Starting 9 tests from 4 test cases.
> +1..11
> +# Starting 11 tests from 6 test cases.
>  #  RUN           global.standalone_pass ...
>  # harness-selftest.c:19:standalone_pass:before
>  # harness-selftest.c:23:standalone_pass:after
> @@ -60,5 +60,19 @@ ok 8 fixture_parent.pass
>  # pass: Test terminated by assertion
>  #          FAIL  fixture_setup_failure.pass
>  not ok 9 fixture_setup_failure.pass
> -# FAILED: 4 / 9 tests passed.
> -# Totals: pass:4 fail:5 xfail:0 xpass:0 skip:0 error:0
> +#  RUN           fixture_variant.v32.pass ...
> +# harness-selftest.c:141:pass:setup 32
> +# harness-selftest.c:150:pass:before
> +# harness-selftest.c:152:pass:after
> +# harness-selftest.c:146:pass:teardown same-process=1
> +#            OK  fixture_variant.v32.pass
> +ok 10 fixture_variant.v32.pass
> +#  RUN           fixture_variant.v64.pass ...
> +# harness-selftest.c:141:pass:setup 64
> +# harness-selftest.c:150:pass:before
> +# harness-selftest.c:152:pass:after
> +# harness-selftest.c:146:pass:teardown same-process=1
> +#            OK  fixture_variant.v64.pass
> +ok 11 fixture_variant.v64.pass
> +# FAILED: 6 / 11 tests passed.
> +# Totals: pass:6 fail:5 xfail:0 xpass:0 skip:0 error:0
> -- 
> 2.34.1
> 

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Wei Yang]

On Tue, Jun 17, 2025 at 09:35:12AM +0200, Thomas Weißschuh wrote:
>Good idea.
>

Thanks.

>On Mon, Jun 16, 2025 at 12:23:38PM +0000, Wei Yang wrote:
>> Each fixture could support variant. Add fixture with variant to verify
>> the behavior, so we can validate for further change.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> ---
>>  .../kselftest_harness/harness-selftest.c      | 34 +++++++++++++++++++
>>  .../harness-selftest.expected                 | 22 +++++++++---
>>  2 files changed, 52 insertions(+), 4 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/kselftest_harness/harness-selftest.c b/tools/testing/selftests/kselftest_harness/harness-selftest.c
>> index b555493bdb4d..2fd5310b33c7 100644
>> --- a/tools/testing/selftests/kselftest_harness/harness-selftest.c
>> +++ b/tools/testing/selftests/kselftest_harness/harness-selftest.c
>> @@ -118,6 +118,40 @@ TEST_F(fixture_setup_failure, pass) {
>>  	TH_LOG("after");
>>  }
>>  
>> +FIXTURE(fixture_variant) {
>> +	pid_t testpid;
>> +};
>> +
>> +FIXTURE_VARIANT(fixture_variant)
>> +{
>> +	int value;
>> +};
>> +
>> +FIXTURE_VARIANT_ADD(fixture_variant, v32)
>> +{
>> +	.value = 32,
>> +};
>> +
>> +FIXTURE_VARIANT_ADD(fixture_variant, v64)
>> +{
>> +	.value = 64,
>> +};
>> +
>> +FIXTURE_SETUP(fixture_variant) {
>> +	TH_LOG("setup %d", variant->value);
>> +	self->testpid = getpid();
>> +}
>> +
>> +FIXTURE_TEARDOWN(fixture_variant) {
>> +	TH_LOG("teardown same-process=%d", self->testpid == getpid());
>> +}
>> +
>> +TEST_F(fixture_variant, pass) {
>> +	TH_LOG("before");
>> +	ASSERT_EQ(0, 0);
>
>Please log the variant value from the test itself and the teardown function.
>Also I don't think we need the pid logging and before/after/ASSERT in this test
>also, it is already validated in the other ones.
>

Sure, per my understanding, is this what you prefer?


FIXTURE_SETUP(fixture_variant) {
	TH_LOG("setup %d", variant->value);
}

FIXTURE_TEARDOWN(fixture_variant) {
	TH_LOG("teardown %d", variant->value);
}

TEST_F(fixture_variant, pass) {
	TH_LOG("before %d", variant->value);
	ASSERT_EQ(0, 0);
	TH_LOG("after %d", variant->value);
}


-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Thomas Weißschuh]

On Tue, Jun 17, 2025 at 11:57:48PM +0000, Wei Yang wrote:
> On Tue, Jun 17, 2025 at 09:35:12AM +0200, Thomas Weißschuh wrote:

<snip>

> >> +FIXTURE_SETUP(fixture_variant) {
> >> +	TH_LOG("setup %d", variant->value);
> >> +	self->testpid = getpid();
> >> +}
> >> +
> >> +FIXTURE_TEARDOWN(fixture_variant) {
> >> +	TH_LOG("teardown same-process=%d", self->testpid == getpid());
> >> +}
> >> +
> >> +TEST_F(fixture_variant, pass) {
> >> +	TH_LOG("before");
> >> +	ASSERT_EQ(0, 0);
> >
> >Please log the variant value from the test itself and the teardown function.
> >Also I don't think we need the pid logging and before/after/ASSERT in this test
> >also, it is already validated in the other ones.
> >
> 
> Sure, per my understanding, is this what you prefer?
> 
> 
> FIXTURE_SETUP(fixture_variant) {
> 	TH_LOG("setup %d", variant->value);
> }
> 
> FIXTURE_TEARDOWN(fixture_variant) {
> 	TH_LOG("teardown %d", variant->value);
> }
> 
> TEST_F(fixture_variant, pass) {
> 	TH_LOG("before %d", variant->value);
> 	ASSERT_EQ(0, 0);
> 	TH_LOG("after %d", variant->value);

I would drop the three lines above and just do:

TH_LOG("test function %d", variant->value);

Also please note that my earlier comment about the patch prefix
"selftests: harness:" should only apply to the patches really related to the
harness.
Not patch 2, which should use "selftests: kselftest:".

> }
> 
> 
> -- 
> Wei Yang
> Help you, Help me

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Wei Yang]

On Wed, Jun 18, 2025 at 07:47:19AM +0200, Thomas Weißschuh wrote:
>On Tue, Jun 17, 2025 at 11:57:48PM +0000, Wei Yang wrote:
>> On Tue, Jun 17, 2025 at 09:35:12AM +0200, Thomas Weißschuh wrote:
>
><snip>
>
>> >> +FIXTURE_SETUP(fixture_variant) {
>> >> +	TH_LOG("setup %d", variant->value);
>> >> +	self->testpid = getpid();
>> >> +}
>> >> +
>> >> +FIXTURE_TEARDOWN(fixture_variant) {
>> >> +	TH_LOG("teardown same-process=%d", self->testpid == getpid());
>> >> +}
>> >> +
>> >> +TEST_F(fixture_variant, pass) {
>> >> +	TH_LOG("before");
>> >> +	ASSERT_EQ(0, 0);
>> >
>> >Please log the variant value from the test itself and the teardown function.
>> >Also I don't think we need the pid logging and before/after/ASSERT in this test
>> >also, it is already validated in the other ones.
>> >
>> 
>> Sure, per my understanding, is this what you prefer?
>> 
>> 
>> FIXTURE_SETUP(fixture_variant) {
>> 	TH_LOG("setup %d", variant->value);
>> }
>> 
>> FIXTURE_TEARDOWN(fixture_variant) {
>> 	TH_LOG("teardown %d", variant->value);
>> }
>> 
>> TEST_F(fixture_variant, pass) {
>> 	TH_LOG("before %d", variant->value);
>> 	ASSERT_EQ(0, 0);
>> 	TH_LOG("after %d", variant->value);
>
>I would drop the three lines above and just do:
>
>TH_LOG("test function %d", variant->value);
>

Got it, thanks.

>Also please note that my earlier comment about the patch prefix
>"selftests: harness:" should only apply to the patches really related to the
>harness.
>Not patch 2, which should use "selftests: kselftest:".
>

Hmm.. for patch 2, Muhammad mentioned it not comply with TAP guideline.

So I plan to drop it in next version.

>> }
>> 
>> 
>> -- 
>> Wei Yang
>> Help you, Help me

-- 
Wei Yang
Help you, Help me

Re: [PATCH 3/3] selftests: harness: Add kselftest harness selftest with variant

[Wei Yang]

On Wed, Jun 18, 2025 at 08:16:53AM +0000, Wei Yang wrote:
[...]
>>
>>TH_LOG("test function %d", variant->value);
>>
>
>Got it, thanks.
>
>>Also please note that my earlier comment about the patch prefix
>>"selftests: harness:" should only apply to the patches really related to the
>>harness.
>>Not patch 2, which should use "selftests: kselftest:".
>>
>
>Hmm.. for patch 2, Muhammad mentioned it not comply with TAP guideline.
>
>So I plan to drop it in next version.
>

If I misunderstand that, please let me know.

-- 
Wei Yang
Help you, Help me