From: David Gow <davidgow@google.com>
To: Daniel Latypov <dlatypov@google.com>
Cc: Brendan Higgins <brendanhiggins@google.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
KUnit Development <kunit-dev@googlegroups.com>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@vger.kernel.org>,
Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH] kunit: tool: yield output from run_kernel in real time
Date: Tue, 5 Oct 2021 07:34:16 +0800 [thread overview]
Message-ID: <CABVgOS=LsVTvX-RnsfE775fnq4aGQt7SUCeRpBpEd03My99NTQ@mail.gmail.com> (raw)
In-Reply-To: <20211004170102.2522514-1-dlatypov@google.com>
On Tue, Oct 5, 2021 at 1:01 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Currently, `run_kernel()` dumps all the kernel output to a file
> (.kunit/test.log) and then opens the file and yields it to callers.
> This made it easier to respect the requested timeout, if any.
>
> But it means that we can't yield the results in real time, either to the
> parser or to stdout (if --raw_output is set).
>
> This change spins up a background thread to enforce the timeout, which
> allows us to yield the kernel output in real time, while also copying it
> to the .kunit/test.log file.
> It's also careful to ensure that the .kunit/test.log file is complete,
> even in the kunit_parser throws an exception/otherwise doesn't consume
> every line, see the new `finally` block and unit test.
>
> For example:
>
> $ ./tools/testing/kunit/kunit.py run --arch=x86_64 --raw_output
> <configure + build steps>
> ...
> <can now see output from QEMU in real time>
>
> This does not currently have a visible effect when --raw_output is not
> passed, as kunit_parser.py currently only outputs everything at the end.
> But that could change, and this patch is a necessary step towards
> showing parsed test results in real time.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
> tools/testing/kunit/kunit_kernel.py | 73 +++++++++++++++-----------
> tools/testing/kunit/kunit_tool_test.py | 17 ++++++
> 2 files changed, 60 insertions(+), 30 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 2c6f916ccbaf..b8cba8123aa3 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -12,7 +12,8 @@ import subprocess
> import os
> import shutil
> import signal
> -from typing import Iterator, Optional, Tuple
> +import threading
> +from typing import Iterator, List, Optional, Tuple
>
> from contextlib import ExitStack
>
> @@ -103,8 +104,8 @@ class LinuxSourceTreeOperations(object):
> if stderr: # likely only due to build warnings
> print(stderr.decode())
>
> - def run(self, params, timeout, build_dir, outfile) -> None:
> - pass
> + def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
> + raise RuntimeError('not implemented!')
>
>
> class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> @@ -123,7 +124,7 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> kconfig.parse_from_string(self._kconfig)
> base_kunitconfig.merge_in_entries(kconfig)
>
> - def run(self, params, timeout, build_dir, outfile):
> + def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
> kernel_path = os.path.join(build_dir, self._kernel_path)
> qemu_command = ['qemu-system-' + self._qemu_arch,
> '-nodefaults',
> @@ -134,18 +135,10 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> '-nographic',
> '-serial stdio'] + self._extra_qemu_params
> print('Running tests with:\n$', ' '.join(qemu_command))
> - with open(outfile, 'w') as output:
> - process = subprocess.Popen(' '.join(qemu_command),
> - stdin=subprocess.PIPE,
> - stdout=output,
> - stderr=subprocess.STDOUT,
> - text=True, shell=True)
> - try:
> - process.wait(timeout=timeout)
> - except Exception as e:
> - print(e)
> - process.terminate()
> - return process
> + return subprocess.Popen(' '.join(qemu_command),
> + stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT,
> + text=True, shell=True)
>
> class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> """An abstraction over command line operations performed on a source tree."""
> @@ -175,17 +168,13 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> kunit_parser.print_with_timestamp(
> 'Starting Kernel with all configs takes a few minutes...')
>
> - def run(self, params, timeout, build_dir, outfile):
> + def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
> """Runs the Linux UML binary. Must be named 'linux'."""
> linux_bin = get_file_path(build_dir, 'linux')
> - outfile = get_outfile_path(build_dir)
> - with open(outfile, 'w') as output:
> - process = subprocess.Popen([linux_bin] + params,
> - stdin=subprocess.PIPE,
This breaks --raw_output under UML for me. Including the
stdin=subprocess.PIPE again seems to fix it.
> - stdout=output,
> - stderr=subprocess.STDOUT,
> - text=True)
> - process.wait(timeout)
> + return subprocess.Popen([linux_bin] + params,
> + stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT,
> + text=True)
>
> def get_kconfig_path(build_dir) -> str:
> return get_file_path(build_dir, KCONFIG_PATH)
> @@ -330,12 +319,36 @@ class LinuxSourceTree(object):
> args.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt'])
> if filter_glob:
> args.append('kunit.filter_glob='+filter_glob)
> - outfile = get_outfile_path(build_dir)
> - self._ops.run(args, timeout, build_dir, outfile)
> - subprocess.call(['stty', 'sane'])
> - with open(outfile, 'r') as file:
> - for line in file:
> +
> + process = self._ops.start(args, build_dir)
> + assert process.stdout is not None # tell mypy it's set
> +
> + # Enforce the timeout in a background thread.
> + def _wait_proc():
> + try:
> + process.wait(timeout=timeout)
> + except Exception as e:
> + print(e)
> + process.terminate()
> + process.wait()
> + waiter = threading.Thread(target=_wait_proc)
> + waiter.start()
> +
> + output = open(get_outfile_path(build_dir), 'w')
> + try:
> + # Tee the output to the file and to our caller in real time.
> + for line in process.stdout:
> + output.write(line)
> yield line
> + # This runs even if our caller doesn't consume every line.
> + finally:
> + # Flush any leftover output to the file
> + output.write(process.stdout.read())
> + output.close()
> + process.stdout.close()
> +
> + waiter.join()
> + subprocess.call(['stty', 'sane'])
>
> def signal_handler(self, sig, frame) -> None:
> logging.error('Build interruption occurred. Cleaning console.')
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 619c4554cbff..f9a7398a9584 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -14,6 +14,7 @@ import tempfile, shutil # Handling test_tmpdir
> import itertools
> import json
> import signal
> +import subprocess
> import os
>
> import kunit_config
> @@ -291,6 +292,22 @@ class LinuxSourceTreeTest(unittest.TestCase):
> pass
> tree = kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir)
>
> + def test_run_kernel_hits_exception(self):
> + def fake_start(unused_args, unused_build_dir):
> + return subprocess.Popen(['echo "hi\nbye"'], shell=True, text=True, stdout=subprocess.PIPE)
> +
> + with tempfile.TemporaryDirectory('') as build_dir:
> + tree = kunit_kernel.LinuxSourceTree(build_dir, load_config=False)
> + mock.patch.object(tree._ops, 'start', side_effect=fake_start).start()
> +
> + with self.assertRaises(ValueError):
> + for line in tree.run_kernel(build_dir=build_dir):
> + self.assertEqual(line, 'hi\n')
> + raise ValueError('uh oh, did not read all output')
> +
> + with open(kunit_kernel.get_outfile_path(build_dir), 'rt') as outfile:
> + self.assertEqual(outfile.read(), 'hi\nbye\n', msg='Missing some output')
> +
> # TODO: add more test cases.
>
>
>
> base-commit: 3b29021ddd10cfb6b2565c623595bd3b02036f33
> --
> 2.33.0.800.g4c38ced690-goog
>
next prev parent reply other threads:[~2021-10-04 23:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-04 17:01 [PATCH] kunit: tool: yield output from run_kernel in real time Daniel Latypov
2021-10-04 23:34 ` David Gow [this message]
2021-10-04 23:46 ` Daniel Latypov
2021-10-04 23:58 ` David Gow
2021-10-05 0:36 ` Daniel Latypov
2021-10-05 1:08 ` David Gow
2021-10-05 1:17 ` Daniel Latypov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CABVgOS=LsVTvX-RnsfE775fnq4aGQt7SUCeRpBpEd03My99NTQ@mail.gmail.com' \
--to=davidgow@google.com \
--cc=brendanhiggins@google.com \
--cc=dlatypov@google.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).