diff mbox series

[1/3] KVM: selftests: Use TAP interface in the kvm_binary_stats_test

Message ID 20221004093131.40392-2-thuth@redhat.com
State Accepted
Commit d324a733a9346081957f9d74b9e0a05e8a7a0da5
Headers show
Series Use TAP in some more KVM selftests | expand

Commit Message

Thomas Huth Oct. 4, 2022, 9:31 a.m. UTC
The kvm_binary_stats_test test currently does not have any output (unless
one of the TEST_ASSERT statement fails), so it's hard to say for a user
how far it did proceed already. Thus let's make this a little bit more
user-friendly and include some TAP output via the kselftest.h interface.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tools/testing/selftests/kvm/kvm_binary_stats_test.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Andrew Jones Oct. 5, 2022, 8:33 a.m. UTC | #1
On Tue, Oct 04, 2022 at 11:31:29AM +0200, Thomas Huth wrote:
> The kvm_binary_stats_test test currently does not have any output (unless
> one of the TEST_ASSERT statement fails), so it's hard to say for a user
> how far it did proceed already. Thus let's make this a little bit more
> user-friendly and include some TAP output via the kselftest.h interface.

I like the idea of switching the entire kvm selftests framework and all
tests to the ksft TAP interface. But, if we want to do that, then the
question is whether we should start by partially using it for some tests,
and then eventually switch over the framework, or whether we should try to
switch everything at once.

I think I prefer the latter, because without changing the framework we
can't provide full TAP anyway as TEST_ASSERT exits with 254 instead of 1
for a fail-exit and it doesn't output a final TAP test summary either.

All that said, this patch doesn't make the current situation worse and
it may help inspire the grand conversion, so

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Thanks,
drew
Sean Christopherson Oct. 7, 2022, 1:22 a.m. UTC | #2
On Wed, Oct 05, 2022, Andrew Jones wrote:
> On Tue, Oct 04, 2022 at 11:31:29AM +0200, Thomas Huth wrote:
> > The kvm_binary_stats_test test currently does not have any output (unless
> > one of the TEST_ASSERT statement fails), so it's hard to say for a user
> > how far it did proceed already. Thus let's make this a little bit more
> > user-friendly and include some TAP output via the kselftest.h interface.
> 
> I like the idea of switching the entire kvm selftests framework and all
> tests to the ksft TAP interface. But, if we want to do that, then the
> question is whether we should start by partially using it for some tests,
> and then eventually switch over the framework, or whether we should try to
> switch everything at once.
> 
> I think I prefer the latter, because without changing the framework we
> can't provide full TAP anyway as TEST_ASSERT exits with 254 instead of 1
> for a fail-exit and it doesn't output a final TAP test summary either.

I would much prefer the latter, e.g. with common entry/exit points[*], much of the
boilerplate can be done once in common code.  I bet we could even figure out a way
to have tests default to setting a plan of '1' so that simple tests don't need to
care about TAP at all.

[*] https://lore.kernel.org/all/20220915000448.1674802-2-vannapurve@google.com
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/kvm_binary_stats_test.c b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
index 0b45ac593387..894417c96f70 100644
--- a/tools/testing/selftests/kvm/kvm_binary_stats_test.c
+++ b/tools/testing/selftests/kvm/kvm_binary_stats_test.c
@@ -19,6 +19,7 @@ 
 #include "kvm_util.h"
 #include "asm/kvm.h"
 #include "linux/kvm.h"
+#include "kselftest.h"
 
 static void stats_test(int stats_fd)
 {
@@ -51,7 +52,7 @@  static void stats_test(int stats_fd)
 
 	/* Sanity check for other fields in header */
 	if (header.num_desc == 0) {
-		printf("No KVM stats defined!");
+		ksft_print_msg("No KVM stats defined!\n");
 		return;
 	}
 	/*
@@ -224,9 +225,13 @@  int main(int argc, char *argv[])
 			max_vcpu = DEFAULT_NUM_VCPU;
 	}
 
+	ksft_print_header();
+
 	/* Check the extension for binary stats */
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_BINARY_STATS_FD));
 
+	ksft_set_plan(max_vm);
+
 	/* Create VMs and VCPUs */
 	vms = malloc(sizeof(vms[0]) * max_vm);
 	TEST_ASSERT(vms, "Allocate memory for storing VM pointers");
@@ -245,10 +250,12 @@  int main(int argc, char *argv[])
 		vm_stats_test(vms[i]);
 		for (j = 0; j < max_vcpu; ++j)
 			vcpu_stats_test(vcpus[i * max_vcpu + j]);
+		ksft_test_result_pass("vm%i\n", i);
 	}
 
 	for (i = 0; i < max_vm; ++i)
 		kvm_vm_free(vms[i]);
 	free(vms);
-	return 0;
+
+	ksft_finished();	/* Print results and exit() accordingly */
 }