diff mbox series

kunit: tool: print parsed test results fully incrementally

Message ID 20211007054410.290427-1-dlatypov@google.com (mailing list archive)
State New
Delegated to: Brendan Higgins
Headers show
Series kunit: tool: print parsed test results fully incrementally | expand

Commit Message

Daniel Latypov Oct. 7, 2021, 5:44 a.m. UTC 
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@google.com/
[2] https://lore.kernel.org/linux-kselftest/20211005011340.2826268-1-dlatypov@google.com/

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 tools/testing/kunit/kunit_parser.py    | 22 ++++++++++----
 tools/testing/kunit/kunit_tool_test.py | 42 +++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 7 deletions(-)


base-commit: 9b409050eaf2da929408fa60fbf535745d828e67

Comments

David Gow Oct. 7, 2021, 6:55 a.m. UTC | #1 
On Thu, Oct 7, 2021 at 1:44 PM Daniel Latypov <dlatypov@google.com> 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@google.com/
> [2] https://lore.kernel.org/linux-kselftest/20211005011340.2826268-1-dlatypov@google.com/
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---

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@google.com>

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
>
diff mbox series

Patch

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):