-
Notifications
You must be signed in to change notification settings - Fork 217
parser: Add KTAP parser for handling kernel selftests #6597
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
parser: Add KTAP parser for handling kernel selftests #6597
Conversation
d3ce481
to
e2f36ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test in t/30-test_parser.t along with an example output?
If you have trouble with the test, let us know, but an example output would be helpful
absolutely, I will try to add such test and if I find this difficult I will use your offer. Thanks for a quick check! I appreciate it |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6597 +/- ##
=======================================
Coverage 99.11% 99.12%
=======================================
Files 399 400 +1
Lines 40723 40809 +86
=======================================
+ Hits 40364 40450 +86
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e2f36ba
to
fb69c04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only have some style nitpicks and would also suggest to use signatures.
We probably also want 100 % test coverage for newly introduced code so unit tests need to be extended.
Looks like the CI check for patch coverage is passing. This is probably because this change introduces a completely new file which Codecov doesn't seem to handle, e.g. the new file is not present in the listing on https://app.codecov.io/gh/os-autoinst/openQA/pull/6597/tree/lib/OpenQA/Parser/Format?dropdown=coverage. (Making the URL manually leads to https://app.codecov.io/gh/os-autoinst/openQA/pull/6597/blob/lib/OpenQA/Parser/Format/KTAP.pm?dropdown=coverage showing no coverage, though.) |
sub ktap_parse added. I admit its the first time I write unit tests in perl let alone openQA project :-) This looks ok: and if I assert against not expected values I get a proper error, so I assume this is OKish. But certainly I would appreciate your review and scrutiny. Thx |
ah 100% I see; I added some test coverage, let me know if this is sufficient or I shall add more. |
thx @okurz for detailed review. I will wait to see if the unit tests are considered good and fix those first if needed and then will start to address your comments taking the advantage of the newly introduced unit tests. |
I would guess this is something what I could, and probably should, "fix" in my PR, so the KTAP.pm would be also checked. |
You've already fixed that as far as your PR is concerned by having some tests at all for the newly introduced file. This is enough for the file to be considered by codecov. You can now use the report to see what code paths still need to be covered. (Whether we want to improve the previous situation where a newly introduced file without any tests still gets a passing coverage CI check is out of scope for this PR. Maybe this is a limitation of codecov we cannot do much about anyway.) |
63923d6
to
9c935f5
Compare
15cb7ae
to
3ddcc08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This will be merged automatically as soon as the patch coverage reports 100%. Maybe codecov later re-evaluates and this fixes itself. Otherwise I wouldn't rush it and wait for any additional idea from any reviewer to find out what's missing here.
FYI: I'm learning more about the unit tests in perl, so I'm pushing some other tests to get the 100% coverage. Thx for the review @okurz |
This patch introduces a dedicated parser for Linux kernel selftests using the KTAP (Kernel Test Anything Protocol) format. While KTAP is based on the TAP (Test Anything Protocol) standard, it includes significant extensions tailored for kernel testing. This parser builds on TAP::Parser to handle common TAP elements while providing additional logic to interpret KTAP-specific constructs such as grouped subtests and prefixed comment lines. For more details on KTAP, see: https://docs.kernel.org/dev-tools/ktap.html Example output of the ktap test run: TAP version 13 1..13 # overriding timeout to 300 # selftests: cgroup: test_core # ok 1 test_cgcore_internal_process_constraint # ok 2 test_cgcore_top_down_constraint_enable # ok 3 test_cgcore_top_down_constraint_disable # ok 4 test_cgcore_no_internal_process_constraint_on_threads # ok 5 test_cgcore_parent_becomes_threaded # ok 6 test_cgcore_invalid_domain # ok 7 test_cgcore_populated # ok 8 test_cgcore_proc_migration # ok 9 test_cgcore_thread_migration # ok 10 test_cgcore_destroy # ok 11 test_cgcore_lesser_euid_open # ok 12 test_cgcore_lesser_ns_open ok 1 selftests: cgroup: test_core There are group of tests with nested subtests. Each group starts as follow: # selftests: cgroup: test_cpu After that individual tests are listed with results ok | not ok: # ok 1 test_cgcore_internal_process_constraint however such subtests are prepended with # which - from the TAP specification standpoint - are just comments. Moreover KTAP allows for more comments like: # overriding timeout to 300 Each group of tests ends with: ok 2 selftests: cgroup: test_cpu or not ok 10 selftests: cgroup: test_zswap # exit=1 The KTAP parser correctly identifies these groups and subtests, associates results appropriately, and exports them in a structured format compatible with OpenQA.
3e4c084
to
0ec6538
Compare
|
||
has [qw(test)]; | ||
|
||
sub _testgroup_init ($self, $line, $test_ref, $steps_ref, $m_ref) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering if instead of having to pass all those references, they can be made members of $self
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. I will try to do that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh really merged instantly once all checks are green.
I will not take it easy @perlpunk and will try it regardless.
Two files with examplary output are added, one with version and plan and the other one without version. Examples contain the group of tests with pass, fail and skip statuses
0ec6538
to
7af7bb3
Compare
Progress: https://progress.opensuse.org/issues/182360
VR: https://rmarliere-openqa.qe.prg2.suse.org/tests/1332
This patch introduces a dedicated parser for Linux kernel selftests using the KTAP (Kernel Test Anything Protocol) format.
While KTAP is based on the TAP (Test Anything Protocol) standard, it includes significant extensions tailored for kernel testing. This parser builds on TAP::Parser to handle common TAP elements while providing additional logic to interpret KTAP-specific constructs such as grouped subtests and prefixed comment lines.
For more details on KTAP, see: https://docs.kernel.org/dev-tools/ktap.html
Example output of the ktap test run:
There are group of tests with nested subtests. Each group starts as follow:
After that individual tests are listed with results ok | not ok:
however such subtests are prepended with # which - from the TAP specification standpoint - are just comments. Moreover KTAP allows for more comments like:
Each group of tests ends with:
or
The KTAP parser correctly identifies these groups and subtests, associates results appropriately, and exports them in a structured format compatible with OpenQA.