Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Oct 2022 21:03:59 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Thomas Huth <thuth@redhat.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org,
        Shuah Khan <shuah@kernel.org>, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 0/3] Use TAP in some more KVM selftests
Message-ID: <Y0nOv6fqTe2NnPuu@google.com>
References: <20221004093131.40392-1-thuth@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221004093131.40392-1-thuth@redhat.com>
Precedence: bulk

On Tue, Oct 04, 2022, Thomas Huth wrote:
> Many KVM selftests are completely silent. This has the disadvantage
> for the users that they do not know what's going on here. For example,
> some time ago, a tester asked me how to know whether a certain new
> sub-test has been added to one of the s390x test binaries or not (which
> he didn't compile on his own), which is hard to judge when there is no
> output. So I finally went ahead and implemented TAP output in the
> s390x-specific tests some months ago.
> 
> Now I wonder whether that could be a good strategy for the x86 and
> generic tests, too?

Taking Andrew's thoughts a step further, I'm in favor of adding TAP output, but
only if we implement it in such a way that it reduces the burden on writing new
tests.  I _really_ like that sync_regs_test's subtests are split into consumable
chunks, but I worry that the amount of boilerplate needed will deter test writes
and increase the maintenance cost.

And my experience with KVM-unit-tests is that letting test writers specify strings
for test names is a bad idea, e.g. using an arbitrary string creates a disconnect
between what the user sees and what code is running, and makes it unnecessarily
difficult to connect a failure back to code.  And if we ever support running
specific testcases by name (I'm still not sure this is a net positive), arbitrary
strings get really annoying because inevitably an arbitrary string will contain
characters that need to be escaped in the shell.

Adding a macro or three to let tests define and run testscases with minimal effort
would more or less eliminate the boilerplate.  And in theory providing semi-rigid
macros would help force simple tests to conform to standard patterns, which should
reduce the cost of someone new understanding the test, and would likely let us do
more automagic things in the future.

E.g. something like this in the test:

	KVM_RUN_TESTCASES(vcpu,
		test_clear_kvm_dirty_regs_bits,
		test_set_invalid,
		test_req_and_verify_all_valid_regs,
		test_set_and_verify_various_reg_values,
		test_clear_kvm_dirty_regs_bits,
	);