Re: [PATCH 2/2] KVM: selftests: Create KVM selftest runner

[Sean Christopherson <seanjc@google.com>]

On Mon, May 05, 2025, Vipin Sharma wrote:
> On 2025-04-30 14:31:05, Sean Christopherson wrote:
> > Printing the timestamps to the console isn't terrible interesting, and IMO isn't
> > at all worth the noise.
> > 
> > The PID is nice, but it needs to be printed _before_ the test finishes, and it
> > needs to track the PID of the test.  If getting that working is non-trivial,
> > definitely punt it for the initial commit.
> > 
> > And presumably INFO is the level of logging.  That needs to go.
> > 
> 
> Instead of removing timestamp, I can just print HH:MM:SS, I think it
> provides value in seeing how fast runner and tests are executing.

I can probably live with that. :-)

> > I think we should also provide controls for the verbosity of the output.  E.g. to
> > skip printing tests that pass entirely.  My vote would be for a collection of
> > boolean knobs, i.e. not a log_level or whatever, because inevitably we'll end up
> > with output that isn't strictly "increasing".
> > 
> > Adding a param to disable printing of passed tests is presumably trivial, so maybe
> > do that for the initial commit, and then we can work on the fancier stuff?
> 
> You mean some command line options like:
> 	testrunner --print-passed --print-failed
> 	testrunner --print-skipped
> 	testrunner --print-timeouts
> 	testrunner --quiet

Ya, something like that.

> I can provide few options in the first commit, and then later we can
> extend it based on usages.

+1 

> > A not-quite-mandatory, but very-nice-to-have feature would be the ability to
> > display which tests Passed/Failed/Skipped/Timed Out/Incomplete, with command line
> > knobs for each.  My vote is for everything but Passed on-by-default, though it's
> > easy enough to put a light wrapper around this (which I'll do no matter what), so
> > my preference for the default doesn't matter all that much.
> > 
> > That could tie into the above idea of grabbing keys to print such information on-demand.
> > 
> 
> This will be very involved feature, lets punt it to a later versions, if
> needed.

Sounds good.

> > The runner should have an (on-by-default?) option to abort if the output directory
> > already exists, e.g. so that users don't clobber previous runs.  And/or an option
> > to append a timestamp, e.g. $result.yyyy.mm.dd.MM.SS, so that all users don't end
> > up writing the same wrapper to generate a timestamp.
> > 
> > Having a no-timestamp + overwrite mode is also useful, e.g. when I'm running in
> > a more "interactive" mode where I'm doing initial testing of something, and I
> > don't care about
> > 
> 
> We can provide user an option like:
> 	testrunner --output result_TIME
> 
> then internally runner will replace TIME with the current time?

Why overload --output and then have to do more parsing?  I assume adding options
is easy, so presumably --append-timestamp would be just as easy to add.

> If user doesn't provide _TIME then we can overwrite the directory
> provided.

I don't see any reason to tie those two together.  Again, on the assumption that
adding options is mechaincally easy, I'd much rather have --overwrite or whatever.

In general, so long as it doesn't meaningfully increase complexity, make the
interface as flexible as possible so that the runner has a decent chance of being
able to handle whatever use cases people come up with, without needing constant
tweaking and churn.