On Wed, May 26, 2021 at 1:22 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote:
>
> Note: this does not change the parser behavior at all (except for making
> one error message more useful). This is just an internal refactor.
>
> The TAP output parser currently operates over a List[str].
> This works, but we only ever need to be able to "peek" at the current
> line and the ability to "pop" it off.
>
> Also, using a List means we need to wait for all the output before we
> can start parsing. While this is not an issue for most tests which are
> really lightweight, we do have some longer (~5 minutes) tests.
>
> This patch introduces an LineStream wrapper class that
> * Exposes a peek()/pop() interface instead of manipulating an array
> * this allows us to more easily add debugging code [1]
> * Can consume an input from a generator
> * we can now parse results as tests are running (the parser code
> currently doesn't print until the end, so no impact yet).
> * Tracks the current line number to print better error messages
> * Would allow us to add additional features more easily, e.g. storing
> N previous lines so we can print out invalid lines in context, etc.
>
> [1] The parsing logic is currently quite fragile.
> E.g. it'll often say the kernel "CRASHED" if there's something slightly
> wrong with the output format. When debugging a test that had some memory
> corruption issues, it resulted in very misleading errors from the parser.
>
> Now we could easily add this to trace all the lines consumed and why
> +import inspect
> ...
> def pop(self) -> str:
> n = self._next
> + print(f'popping {n[0]}: {n[1].ljust(40, " ")}| caller={inspect.stack()[1].function}')
>
> Example output:
> popping 77: TAP version 14 | caller=parse_tap_header
> popping 78: 1..1 | caller=parse_test_plan
> popping 79: # Subtest: kunit_executor_test | caller=parse_subtest_header
> popping 80: 1..2 | caller=parse_subtest_plan
> popping 81: ok 1 - parse_filter_test | caller=parse_ok_not_ok_test_case
> popping 82: ok 2 - filter_subsuite_test | caller=parse_ok_not_ok_test_case
> popping 83: ok 1 - kunit_executor_test | caller=parse_ok_not_ok_test_suite
>
> If we introduce an invalid line, we can see the parser go down the wrong path:
> popping 77: TAP version 14 | caller=parse_tap_header
> popping 78: 1..1 | caller=parse_test_plan
> popping 79: # Subtest: kunit_executor_test | caller=parse_subtest_header
> popping 80: 1..2 | caller=parse_subtest_plan
> popping 81: 1..2 # this is invalid! | caller=parse_ok_not_ok_test_case
> popping 82: ok 1 - parse_filter_test | caller=parse_ok_not_ok_test_case
> popping 83: ok 2 - filter_subsuite_test | caller=parse_ok_not_ok_test_case
> popping 84: ok 1 - kunit_executor_test | caller=parse_ok_not_ok_test_case
> [ERROR] ran out of lines before end token
>
> Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx>
> Reviewed-by: David Gow <davidgow@xxxxxxxxxx>
Acked-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
