Re: [PATCH v2 03/15] KVM: selftests: Add timeout option in selftests runner

[Sean Christopherson <seanjc@google.com>]

On Fri, Jun 06, 2025, Vipin Sharma wrote:
> diff --git a/tools/testing/selftests/kvm/runner/command.py b/tools/testing/selftests/kvm/runner/command.py
> index a63ff53a92b3..44c8e0875779 100644
> --- a/tools/testing/selftests/kvm/runner/command.py
> +++ b/tools/testing/selftests/kvm/runner/command.py
> @@ -12,14 +12,16 @@ class Command:
>      Returns the exit code, std output and std error of the command.
>      """
>  
> -    def __init__(self, command):
> +    def __init__(self, command, timeout):
>          self.command = command
> +        self.timeout = timeout
>  
>      def run(self):
>          run_args = {
>              "universal_newlines": True,
>              "shell": True,
>              "capture_output": True,
> +            "timeout": self.timeout,
>          }
> @@ -48,10 +50,13 @@ class Selftest:
>              self.stderr = "File doesn't exists."
>              return
>  
> -        ret, self.stdout, self.stderr = self.command.run()
> -        if ret == 0:
> -            self.status = SelftestStatus.PASSED
> -        elif ret == 4:
> -            self.status = SelftestStatus.SKIPPED
> -        else:
> -            self.status = SelftestStatus.FAILED
> +        try:
> +            ret, self.stdout, self.stderr = self.command.run()

I don't see any value in having both command.py and selftest.py.  TimeoutExpired
*really* should be handled by Command, especially with respect to stdout/stderr
(more on that later), but that complicates converting return codes to SelftestStatus.

And even if we do figure out a clean split, one of Command or Selftest would end
but being little more than a wrapper or trampoline, with more boilerplate code
than novel logic.

I don't anticipate turning this into a general execution framework.  The entire
purpose is to run a selftest, so I think it makes sense to scrap Command and just
have Selftest deal with running the executable.

> +            if ret == 0:
> +                self.status = SelftestStatus.PASSED
> +            elif ret == 4:
> +                self.status = SelftestStatus.SKIPPED
> +            else:
> +                self.status = SelftestStatus.FAILED
> +        except subprocess.TimeoutExpired as e:
> +            self.status = SelftestStatus.TIMED_OUT
> diff --git a/tools/testing/selftests/kvm/runner/test_runner.py b/tools/testing/selftests/kvm/runner/test_runner.py
> index 104f0b4c2e4e..1409e1cfe7d5 100644
> --- a/tools/testing/selftests/kvm/runner/test_runner.py
> +++ b/tools/testing/selftests/kvm/runner/test_runner.py
> @@ -15,7 +15,7 @@ class TestRunner:
>          self.tests = []
>  
>          for test_file in test_files:
> -            self.tests.append(Selftest(test_file, args.executable))
> +            self.tests.append(Selftest(test_file, args.executable, args.timeout))
>  
>      def _log_result(self, test_result):
>          logger.log(test_result.status,
> -- 
> 2.50.0.rc0.604.gd4ff7b7c86-goog
>