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(-)
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. */
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
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
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)
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..0Change 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)
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..0Change 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)
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
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
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
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
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); }
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
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
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.
linux-kselftest-mirror@lists.linaro.org
-
Muhammad Usama Anjum -
Thomas Weißschuh -
Wei Yang