tests: kdoc: fix parse failure logging #63
Conversation
|
I figured out the log lines that are just printing numbers. It is a regression in the kernel-doc script, which I submitted a fix here: https://lore.kernel.org/all/20251029-jk-fix-kernel-doc-duplicate-return-warning-v1-1-28ed58bec304@intel.com/T/#u Combined with this fixup I think we should be good again. |
|
Meaning for the test to work we need that fix? There is no way for us to apply a fix locally... |
We could make our script ignore lines like "Warning: " but I don't think that is a great solution, as it papers over the invalid output from kernel-doc. It only triggers false positives for cases with files containing duplicate section names like the virtio.h header, which hopefully aren't too many? Either way, we do need to merge this fix to stop logging parse fail on lines which actually parse just fine. I also had a thought about treating the warnings with just a line number as some sort of count instead, which I think I can get working. Its not ideal but it would work around the broken script somewhat. |
Commit 0a1066f ("tests: kdoc: fix handling file removal") added extra logging to the test intending to help debug parsing failures. Unfortunately, the newly added "<parse fail>" log line is added to every warning which doesn't end in ':', regardless of whether or not the KdocWarning class handles parsing of the line. This appears to have happened due to the confusion of the way skip and extra work. What we really want to do is convert the text into a warning object, then check if it cleanly parsed. We could check for the 'Unknown' kind. However, in the interest of being precise should new kinds ever be added, expose a new 'parsed' field of the warning object. Its set to true if we got a clean regex parse, and false otherwise. Additionally refactor the conditional checks and add comments for clarity. In particular, set extra to None unconditionally first, then perform the merging check. This should improve the readability and intended flow of this logic. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
jacob-keller
force-pushed
the
jk-fix-parse-fail-logging
branch
from
b7c7eb7 to
ccad72e
Compare
Some versions of kernel-doc currently report certain warnings with only
their line number. This breaks the logic of the kdoc test, as such lines do
not pass the expected format defined by the KdocWarning class.
Such warnings appear to indicate duplicate section headers in a kernel doc
entry, but the text of the warning is not properly displayed.
As a result, the kdoc test produces false positive failures for patches
which touch files that have such warnings. The line contents are compared
without ignoring the line numbers. This produces output failures:
Warnings before: 7 after: 7 (add: 2 del: 2)
Warnings removed:
Warning: 174
Warning: 184
Per-file breakdown:
2 None
New warnings added:
Warning: 180
Warning: 196
Per-file breakdown:
2 None
Since the warnings are compared directly, the changing line numbers are
detected as differences.
This is a bug in the kernel-doc script, and a proper fix requires updating
the script to stop producing such warnings. In the mean time, the NIPA
tests continue to fail. To fix that, use a separate regular expression to
identify lines of the form 'Warning: <number>'. Set the content of such
warnings to None.
This will make all such warnings compare as identical in the existing
algorithm, and thus the changing line numbers will no longer cause such a
false positive.
To avoid introducing further regressions, extend the check to count the
total number of unparsed warnings. If it increases, then also fail the kdoc
test. This way, we will correctly fail when we see more empty warnings
after applying the current patch.
This also should help catch issues for other types of unrecognized warnings
in the future. Note that all warnings still get compared in the usual way,
where fully unrecognized warnings will be compare the lines as-is without
eliding the line number comparison. This just adds an additional safety
guard in addition to handling the empty warning case.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
jacob-keller
force-pushed
the
jk-fix-parse-fail-logging
branch
from
ccad72e to
948b6bb
Compare
|
@kuba-moo check this version out. It produces the following on the patch you originally reported the issues on: It should now count the number of Warning: lines and report a warning if that number increases, but otherwise treat all of those as identical. I think thats a reasonable compromise. Eventually kernel-doc script will be fixed to properly log the warning output, but this should help prevent test failures for now. Since we check to make sure the count doesn't increase, at least we'll be able to hopefully prevent any duplicate section names from being merged in the mean time. |
2 participants
Commit 0a1066f ("tests: kdoc: fix handling file removal") added extra
logging to the test intending to help debug parsing failures.
Unfortunately, the newly added "" log line is added to every
warning which doesn't end in ':', regardless of whether or not the
KdocWarning class handles parsing of the line.
This appears to have happened due to the confusion of the way skip and
extra work. What we really want to do is convert the text into a warning
object, then check if it cleanly parsed. We could check for the 'Unknown'
kind. However, in the interest of being precise should new kinds ever be
added, expose a new 'parsed' field of the warning object. Its set to true
if we got a clean regex parse, and false otherwise.
Additionally refactor the conditional checks and add comments for clarity.
In particular, set extra to None unconditionally first, then perform the
merging check. This should improve the readability and intended flow of
this logic.
Signed-off-by: Jacob Keller jacob.e.keller@intel.com