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

fix: handle reports with line 0 executed #73

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

giovanni-guidini
Copy link
Contributor

Recently when testing the docs for integrating ATS I did this commit
for which processing failed. Checking the logs it failed because of

  [... removed stack trace...]
  File "/worker/services/report/languages/pycoverage.py", line 56, in process
    report_file.append(
  File "/usr/local/lib/python3.10/site-packages/shared/reports/resources.py", line 341, in append
    raise ValueError("Line number must be greater then 0. Got %s" % ln)
ValueError: Line number must be greater then 0. Got 0

Indeed when I donwloaded and checked the uploaded files there were 3 of them with executed_lines: [0].
They were all __init__.py files. Idk why that happened, to be honest. It looks strange because the 3 files are empty.

In any case it's an easy fix that will not hurt us.
I think line 0 should not exist. It might indicate that when importing the code or something like that the file was
executed somehow, but for __init__.py files with some content in them the list of lines executed is as expected.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Recently when testing the docs for integrating ATS I did [this commit](https://app.codecov.io/gh/giovanni-guidini/ats-data/commit/c24c1db8aa27fcda2d311fdcd293c20697030389)
for which processing failed. Checking the logs it failed because of

```
  [... removed stack trace...]
  File "/worker/services/report/languages/pycoverage.py", line 56, in process
    report_file.append(
  File "/usr/local/lib/python3.10/site-packages/shared/reports/resources.py", line 341, in append
    raise ValueError("Line number must be greater then 0. Got %s" % ln)
ValueError: Line number must be greater then 0. Got 0
```

Indeed when I donwloaded and checked the uploaded files there were 3 of them with `executed_lines: [0]`.
They were all `__init__.py` files. Idk why that happened, to be honest. It looks strange because the 3 files are empty.

In any case it's an easy fix that will not hurt us.
I think line 0 should not exist. It might indicate that when importing the code or something like that the file was
executed somehow, but for `__init__.py` files with some content in them the list of lines executed is as expected.
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #73 (6eefde5) into main (ae66235) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main      #73   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files         359      359           
  Lines       26283    26284    +1     
=======================================
+ Hits        25913    25914    +1     
  Misses        370      370           
Flag Coverage Δ
integration 98.56% <100.00%> (+<0.01%) ⬆️
latest-uploader-overall 98.56% <100.00%> (+<0.01%) ⬆️
onlysomelabels 98.58% <100.00%> (+<0.01%) ⬆️
unit 98.56% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
NonTestCode 97.29% <100.00%> (+<0.01%) ⬆️
OutsideTasks 98.34% <100.00%> (+<0.01%) ⬆️
Files Changed Coverage Δ
...ces/report/languages/tests/unit/test_pycoverage.py 100.00% <ø> (ø)
services/report/languages/pycoverage.py 100.00% <100.00%> (ø)
Related Entrypoints
run/app.tasks.upload.UploadProcessor

Copy link
Contributor

@matt-codecov matt-codecov left a comment

Choose a reason for hiding this comment

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

looks like executed_lines comes from coverage.py's json format, and far as i can tell, a line number of 0 is a bug in coverage.py

https://github.com/nedbat/coveragepy/blob/c2b238e86cade2e6163b24656a7d0549f4508e8f/coverage/sqldata.py#L937 this function appears to filter out 0s in one path but not the other. no idea what the root cause is of the 0s is, but i take that as evidence that 0s are not supposed to be returned

if you still have the bugged coverage.json, maybe file an issue on https://github.com/nedbat/coveragepy to let the author know

@giovanni-guidini giovanni-guidini merged commit 4ce0025 into main Aug 22, 2023
12 checks passed
@giovanni-guidini giovanni-guidini deleted the gio/fix-pycoverage branch August 22, 2023 12:41
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.

None yet

2 participants