Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automated: kselftest: Rewrite parser to support nesting #493

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

ryanhrob
Copy link

The previous kselftests TAP parser has a few shortcomings that prevent it from working well with the mm kselftests output. mm kselftests are nested up to 3 levels deep, with the most important information, from the user's perspective, usually contained in the middle level. But the parser isn't nesting-aware, and instead flattens test results to include the first and last level of nesting and ignroed everything in the middle. This leads to undescriptive and confusing test names in kernelci UI.

Additionally, conflicting test names extracted by the parser are not made unique, which leads to multiple distinct tests being collapsed into a single test within the kernelci UI, and its status is set to the last occurence of the test in the list. So you might have multiple instances that fail, but if the last one passes, it is shown as a single test that passes. This problem is compounded by the parser's inability to properly nest because the important middle level information is lost and this makes many more test names look identical, so even more get collapsed into one.

Solve all of this by rewriting the parser to properly support recursive parsing. The tree of tests are then flattened into a test list in depth-first order, where the test name is built from the name of each level. Further, if duplicate test names exist, append a "_dup" to the second instance onwards, where N is a unique number. This guarrantees that all test points output by TAP appear in kernelci UI.

I've tested this against the output for arm64, ftrace, kvm, and sigstack kselftests (which don't have much nesting so work fine with the old parser): The outputs from both parsers are identical, except in a couple of instances where there are duplicate test name outputs and the new parser appends the "_dup" suffix to make it unique.

I've also tested this against the output from the mm kselftests: The output from the new parser is as expected, and much more useful than the old parser.

The downside is that this implementation depends on the tap.py module (https://tappy.readthedocs.io). It is packaged for Debian and Ubuntu, so I've added that package as a dependency. But I couldn't find anything for Centos or Fedora, so this module (and its dependencies) will likely need to be installed from PyPI in these environments:

$ pip3 install tap.py more-itertools pyyaml

The previous kselftests TAP parser has a few shortcomings that prevent
it from working well with the mm kselftests output. mm kselftests are
nested up to 3 levels deep, with the most important information, from
the user's perspective, usually contained in the middle level. But the
parser isn't nesting-aware, and instead flattens test results to
include the first and last level of nesting and ignroed everything in
the middle. This leads to undescriptive and confusing test names in
kernelci UI.

Additionally, conflicting test names extracted by the parser are not
made unique, which leads to multiple distinct tests being collapsed into
a single test within the kernelci UI, and its status is set to the last
occurence of the test in the list. So you might have multiple instances
that fail, but if the last one passes, it is shown as a single test that
passes. This problem is compounded by the parser's inability to properly
nest because the important middle level information is lost and this
makes many more test names look identical, so even more get collapsed
into one.

Solve all of this by rewriting the parser to properly support recursive
parsing. The tree of tests are then flattened into a test list in
depth-first order, where the test name is built from the name of each
level. Further, if duplicate test names exist, append a "_dup<N>" to the
second instance onwards, where N is a unique number. This guarrantees
that all test points output by TAP appear in kernelci UI.

I've tested this against the output for arm64, ftrace, kvm, and sigstack
kselftests (which don't have much nesting so work fine with the old
parser): The outputs from both parsers are identical, except in a couple
of instances where there are duplicate test name outputs and the new
parser appends the "_dup<N>" suffix to make it unique.

I've also tested this against the output from the mm kselftests: The
output from the new parser is as expected, and much more useful than the
old parser.

The downside is that this implementation depends on the tap.py module
(https://tappy.readthedocs.io). It is packaged for Debian and Ubuntu, so
I've added that package as a dependency. But I couldn't find anything
for Centos or Fedora, so this module (and its dependencies) will likely
need to be installed from PyPI in these environments:

  $ pip3 install tap.py more-itertools pyyaml

Signed-off-by: Ryan Roberts <[email protected]>
@ryanhrob
Copy link
Author

I've fixed this up to satisfy the linter. Is it possible to rerun the workflow?

Copy link
Collaborator

@roxell roxell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @ryanhrob for the patch.

@roxell roxell merged commit 6e2cfcd into Linaro:master Feb 16, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants