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

Add a new option to ignore parser errors #788

Merged
merged 2 commits into from
Oct 11, 2023
Merged

Conversation

uhafner
Copy link
Member

@uhafner uhafner commented Oct 11, 2023

For more details, please see jenkinsci/coverage-model#39.

Fixes #785

@uhafner uhafner added bug Bugs or performance problems enhancement Enhancement of existing functionality labels Oct 11, 2023
@uhafner uhafner changed the title Add a new option to ignore parser errors. Add a new option to ignore parser errors Oct 11, 2023
@uhafner uhafner linked an issue Oct 11, 2023 that may be closed by this pull request
@uhafner uhafner removed the enhancement Enhancement of existing functionality label Oct 11, 2023
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #788 (8106422) into master (2ef24aa) will increase coverage by 0.06%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master     #788      +/-   ##
============================================
+ Coverage     74.08%   74.14%   +0.06%     
- Complexity     1698     1705       +7     
============================================
  Files           130      130              
  Lines          6289     6301      +12     
  Branches        677      677              
============================================
+ Hits           4659     4672      +13     
  Misses         1407     1407              
+ Partials        223      222       -1     
Files Coverage Δ
...ugins/coverage/metrics/steps/CoverageRecorder.java 80.85% <100.00%> (+0.63%) ⬆️
.../coverage/metrics/steps/CoverageReportScanner.java 76.92% <100.00%> (+1.92%) ⬆️
...s/plugins/coverage/metrics/steps/CoverageStep.java 70.90% <100.00%> (+1.38%) ⬆️
...s/plugins/coverage/metrics/steps/CoverageTool.java 66.03% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@uhafner uhafner merged commit e2a8b71 into master Oct 11, 2023
27 of 28 checks passed
@uhafner uhafner deleted the ignore-cobertura-errors branch October 11, 2023 15:41
@medianick
Copy link
Contributor

We ran into this issue and I adopted this new ignoreParsingErrors: true option in our recordCoverage calls. That mostly solved the issue we were encountering, but we now transiently get errors like this (despite the setting):

There is already a child [CLASS] My.Redacted.Class`2 <0> with the name My.Redacted.Class`2 in [FILE] RedactedFile.cs <1, LINE: 88.89% (32/36)>

I have to imagine it is the result of the disambiguation issue documented at danielpalme/ReportGenerator#630 but we don't seem to have any way around it, short of rerunning the build and hoping it works. (We could also add try/catch logic around recordCoverage but depend on its output as a status check.)

I'm working on obtaining the specific coverage reports that seem to cause this, but as I noted, it's transient -- rerunning a build of the same commit will sometimes fail and sometimes work.

@uhafner
Copy link
Member Author

uhafner commented Nov 2, 2023

Currently I catch errors on duplicate methods only. I should extend this to duplicate classes as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs or performance problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception for Cobertura reports that contain duplicate methods Coverage reports are not merged line by line
2 participants