On Thu, Oct 7, 2021 at 1:44 PM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > With the parser rework [1] and run_kernel() rework [2], this allows the > parser to print out test results incrementally. > > Currently, that's held up by the fact that the LineStream eagerly > pre-fetches the next line when you call pop(). > This blocks parse_test_result() from returning until the line *after* > the "ok 1 - test name" line is also printed. > > One can see this with the following example: > $ (echo -e 'TAP version 14\n1..3\nok 1 - fake test'; sleep 2; echo -e 'ok 2 - fake test 2'; sleep 3; echo -e 'ok 3 - fake test 3') | ./tools/testing/kunit/kunit.py parse > > Before this patch [1]: there's a pause before 'fake test' is printed. > After this patch: 'fake test' is printed out immediately. > > This patch also adds > * a unit test to verify LineStream's behavior directly > * a test case to ensure that it's lazily calling the generator > * an explicit exception for when users go beyond EOF > > [1] https://lore.kernel.org/linux-kselftest/20211006170049.106852-1-dlatypov@xxxxxxxxxx/ > [2] https://lore.kernel.org/linux-kselftest/20211005011340.2826268-1-dlatypov@xxxxxxxxxx/ > > Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx> > --- Thanks. I tried this out with qemu/i386 and it does appropriately pause while 'time_test_cases' is running, but shows all the results beforehand. For reference, the output (with timestamps): [23:53:06] =============== time_test_cases (1 subtest) ================ [23:53:58] [PASSED] time64_to_tm_test_date_range [23:53:58] ================= [PASSED] time_test_cases ================= Reviewed-by: David Gow <davidgow@xxxxxxxxxx> Cheers, -- David > tools/testing/kunit/kunit_parser.py | 22 ++++++++++---- > tools/testing/kunit/kunit_tool_test.py | 42 +++++++++++++++++++++++++- > 2 files changed, 57 insertions(+), 7 deletions(-) > > diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py > index f01fd565f978..82900a5f9ad6 100644 > --- a/tools/testing/kunit/kunit_parser.py > +++ b/tools/testing/kunit/kunit_parser.py > @@ -172,42 +172,51 @@ class TestCounts: > class LineStream: > """ > A class to represent the lines of kernel output. > - Provides a peek()/pop() interface over an iterator of > + Provides a lazy peek()/pop() interface over an iterator of > (line#, text). > """ > _lines: Iterator[Tuple[int, str]] > _next: Tuple[int, str] > + _need_next: bool > _done: bool > > def __init__(self, lines: Iterator[Tuple[int, str]]): > """Creates a new LineStream that wraps the given iterator.""" > self._lines = lines > self._done = False > + self._need_next = True > self._next = (0, '') > - self._get_next() > > def _get_next(self) -> None: > - """Advances the LineSteam to the next line.""" > + """Advances the LineSteam to the next line, if necessary.""" > + if not self._need_next: > + return > try: > self._next = next(self._lines) > except StopIteration: > self._done = True > + finally: > + self._need_next = False > > def peek(self) -> str: > """Returns the current line, without advancing the LineStream. > """ > + self._get_next() > return self._next[1] > > def pop(self) -> str: > """Returns the current line and advances the LineStream to > the next line. > """ > - n = self._next > - self._get_next() > - return n[1] > + s = self.peek() > + if self._done: > + raise ValueError(f'LineStream: going past EOF, last line was {s}') > + self._need_next = True > + return s > > def __bool__(self) -> bool: > """Returns True if stream has more lines.""" > + self._get_next() > return not self._done > > # Only used by kunit_tool_test.py. > @@ -220,6 +229,7 @@ class LineStream: > > def line_number(self) -> int: > """Returns the line number of the current line.""" > + self._get_next() > return self._next[0] > > # Parsing helper methods: > diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py > index c309ed76aef5..3cb02827c941 100755 > --- a/tools/testing/kunit/kunit_tool_test.py > +++ b/tools/testing/kunit/kunit_tool_test.py > @@ -13,8 +13,9 @@ import tempfile, shutil # Handling test_tmpdir > > import itertools > import json > -import signal > import os > +import signal > +from typing import Iterable > > import kunit_config > import kunit_parser > @@ -320,6 +321,45 @@ class KUnitParserTest(unittest.TestCase): > result.status) > self.assertEqual('kunit-resource-test', result.test.subtests[0].name) > > +def line_stream_from_strs(strs: Iterable[str]) -> kunit_parser.LineStream: > + return kunit_parser.LineStream(enumerate(strs, start=1)) > + > +class LineStreamTest(unittest.TestCase): > + > + def test_basic(self): > + stream = line_stream_from_strs(['hello', 'world']) > + > + self.assertTrue(stream, msg='Should be more input') > + self.assertEqual(stream.line_number(), 1) > + self.assertEqual(stream.peek(), 'hello') > + self.assertEqual(stream.pop(), 'hello') > + > + self.assertTrue(stream, msg='Should be more input') > + self.assertEqual(stream.line_number(), 2) > + self.assertEqual(stream.peek(), 'world') > + self.assertEqual(stream.pop(), 'world') > + > + self.assertFalse(stream, msg='Should be no more input') > + with self.assertRaisesRegex(ValueError, 'LineStream: going past EOF'): > + stream.pop() > + > + def test_is_lazy(self): > + called_times = 0 > + def generator(): > + nonlocal called_times > + for i in range(1,5): > + called_times += 1 > + yield called_times, str(called_times) > + > + stream = kunit_parser.LineStream(generator()) > + self.assertEqual(called_times, 0) > + > + self.assertEqual(stream.pop(), '1') > + self.assertEqual(called_times, 1) > + > + self.assertEqual(stream.pop(), '2') > + self.assertEqual(called_times, 2) > + > class LinuxSourceTreeTest(unittest.TestCase): > > def setUp(self): > > base-commit: 9b409050eaf2da929408fa60fbf535745d828e67 > -- > 2.33.0.882.g93a45727a2-goog >
