-
-
Notifications
You must be signed in to change notification settings - Fork 448
Lcov testing #1289
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
Lcov testing #1289
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
# Licensed under the Apache License: http://www.apache.org/licenses/LICENSE-2.0 | ||
# For details: https://github.com/nedbat/coveragepy/blob/master/NOTICE.txt | ||
|
||
"""LCOV reporting for coverage.py.""" | ||
|
||
import sys | ||
import base64 | ||
from hashlib import md5 | ||
|
||
from coverage.report import get_analysis_to_report | ||
|
||
|
||
class LcovReporter: | ||
"""A reporter for writing LCOV coverage reports.""" | ||
|
||
report_type = "LCOV report" | ||
|
||
def __init__(self, coverage): | ||
self.coverage = coverage | ||
self.config = self.coverage.config | ||
|
||
def report(self, morfs, outfile=None): | ||
"""Renders the full lcov report | ||
|
||
'morfs' is a list of modules or filenames | ||
|
||
outfile is the file object to write the file into. | ||
""" | ||
|
||
self.coverage.get_data() | ||
outfile = outfile or sys.stdout | ||
|
||
for fr, analysis in get_analysis_to_report(self.coverage, morfs): | ||
self.get_lcov(fr, analysis, outfile) | ||
|
||
def get_lcov(self, fr, analysis, outfile=None): | ||
"""Produces the lcov data for a single file | ||
|
||
get_lcov currently supports both line and branch coverage, | ||
however function coverage is not supported. | ||
|
||
""" | ||
|
||
outfile.write("TN:\n") | ||
outfile.write(f"SF:{fr.relative_filename()}\n") | ||
source_lines = fr.source().splitlines() | ||
for covered in sorted(analysis.executed): | ||
# Note: Coveragepy currently only supports checking *if* a line has | ||
# been executed, not how many times, so we set this to 1 for nice | ||
# output even if it's technically incorrect | ||
|
||
# The lines below calculate a 64 bit encoded md5 hash of the line | ||
# corresponding to the DA lines in the lcov file, | ||
# for either case of the line being covered or missed in Coveragepy | ||
# The final two characters of the encoding ("==") are removed from | ||
# the hash to allow genhtml to run on the resulting lcov file | ||
if source_lines: | ||
line = source_lines[covered - 1].encode("utf-8") | ||
else: | ||
line = b"" | ||
hashed = str(base64.b64encode(md5(line).digest())[:-2], encoding="utf-8") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess tastes vary, but I like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But this is where we need to decide what to do about md5: it's not available on FIPS-enabled systems. Here are some choices:
I don't know what the effect would be of omitting the hash, so I don't know which to pick. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The hash is mainly used to ensure the correct lines are marked as covered, to my knowledge. Upstream's Genhtml is of course an unlikely usecase with pycoverage, but it's the reference consumer. It is up to the implementation of the consumer to define what the md5 is used for, but it is marked as optional, so any tools that break on a missing md5 are inherently broken. There shouldn't be any issue with simply omitting the hash when it can't be set, and I can't think of many other uses than "has our user accidentally changed their source". I think that was probably a more common use case around the time lcov added that feature :) It would be sad if pycoverage produced different outputs depending on the machine it is run on. Maybe hence it's best to remove this functionality in general. I think a fifth option is to lobby upstream for supporting a more modern hash, as much as that seems unnecessary. It's honestly a bit silly that python mandates thinking about FIPS compliance with no opt-out if you use its md5 hash, and even more silly that some default python distributions seem to enable FIPS compliance (this seems to be a MacOS thing?). It'll be hard to come up with a backwards-compatible design for that, so I suspect the solution would be to deprecate having hashes at all, since those don't really seem that useful anymore in 2022 when checking for accidental source changes is anyway so easy and often unnecessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the backstory on the hash values.
Is this Python's fault? FIPS forbids md5 as an algorithm.
I haven't encountered this. Can you provide more information? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, not at all. I just think the situation is a bit silly for backwards-compatibility with older formats that use md5 for legitimate reasons (or even just for migration purposes). It's besides the point, sorry for venting here.
This support page seems to suggest a FIPS-compliance mode is enabled by default. Reading further, it may come down to processor support for the hash, rather than something explicitly disabled on the software side, but this is all ultimately besides the point. I think I'll spend a bit more time reading up on where exactly the limitation comes from, and then raise an issue on the lcov upstream to see if they're interested enough in allowing lcov to be run in FIPS-compliant environments. No need for coveragepy to be involved in that :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
outfile.write(f"DA:{covered},1,{hashed}\n") | ||
for missed in sorted(analysis.missing): | ||
if source_lines: | ||
line = source_lines[missed - 1].encode("utf-8") | ||
else: | ||
line = b"" | ||
hashed = str(base64.b64encode(md5(line).digest())[:-2], encoding="utf-8") | ||
outfile.write(f"DA:{missed},0,{hashed}\n") | ||
outfile.write(f"LF:{len(analysis.statements)}\n") | ||
outfile.write(f"LH:{len(analysis.executed)}\n") | ||
|
||
# More information dense branch coverage data | ||
missing_arcs = analysis.missing_branch_arcs() | ||
executed_arcs = analysis.executed_branch_arcs() | ||
for block_number, block_line_number in enumerate( | ||
sorted(analysis.branch_stats().keys()) | ||
): | ||
for branch_number, line_number in enumerate( | ||
sorted(missing_arcs[block_line_number]) | ||
): | ||
# The exit branches have a negative line number, | ||
# this will not produce valid lcov, and so setting | ||
# the line number of the exit branch to 0 will allow | ||
# for valid lcov, while preserving the data | ||
line_number = max(line_number, 0) | ||
outfile.write(f"BRDA:{line_number},{block_number},{branch_number},-\n") | ||
# The start value below allows for the block number to be | ||
# preserved between these two for loops (stopping the loop from | ||
# resetting the value of the block number to 0) | ||
for branch_number, line_number in enumerate( | ||
sorted(executed_arcs[block_line_number]), | ||
start=len(missing_arcs[block_line_number]), | ||
): | ||
line_number = max(line_number, 0) | ||
outfile.write(f"BRDA:{line_number},{block_number},{branch_number},1\n") | ||
|
||
# Summary of the branch coverage | ||
if analysis.has_arcs(): | ||
branch_stats = analysis.branch_stats() | ||
brf = sum(t for t, k in branch_stats.values()) | ||
brh = brf - sum(t - k for t, k in branch_stats.values()) | ||
outfile.write(f"BRF:{brf}\n") | ||
outfile.write(f"BRH:{brh}\n") | ||
|
||
outfile.write("end_of_record\n") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,7 @@ jquery | |
json | ||
jython | ||
kwargs | ||
lcov | ||
Mako | ||
matcher | ||
matchers | ||
|
Uh oh!
There was an error while loading. Please reload this page.