codecov / codecov-cli

Codecov's Command Line Interface. Used for uploading to Codecov in your CI, Test Labelling, Local Upload, and more
Apache License 2.0
56 stars 39 forks source link

[BUG] JUnit test result files are discovered and uploaded as coverage #500

Open webknjaz opened 1 month ago

webknjaz commented 1 month ago

I've been looking into why the newly integrated Codecov app does not post any coverage statuses to GitHub, with all the configuration mimicking what I've been doing in dozens of other repos.

While doing that, I noticed something strange, looking into the upload @ https://app.codecov.io/github/ansible/awx/commit/acd6b2eb2270ff3ebde9b229e0efecb6edee004e. It contains 2 XML files in the payload, while I expected that there would be one file with coverage in it.

So I looked into the job logs @ https://github.com/ansible/awx/actions/runs/10856012453/job/30129820229#step:5:53 and saw that codecov-cli finds 2 files (/home/runner/work/awx/awx/reports/coverage.xml and /home/runner/work/awx/awx/awxkit/report.xml) and decides that both need to be uploaded.

Interestingly, the next CI job step uploads test results and it discovers a different file — /home/runner/work/awx/awx/reports/junit.xml (but not /home/runner/work/awx/awx/awxkit/report.xml) — @ https://github.com/ansible/awx/actions/runs/10856012453/job/30129820229#step:6:34.

~I don't know if that's the reason Codecov can't post statuses to GH (they don't show up for pushed commits, nor PRs), but I figured that it might as well be related.~ UPD: I do know now that it's unrelated.

Anyway, I think this behavior is a bug and the uploader should only look for the files of the correct shape for any given upload type (coverage or test).

My suggestion is that codecov-cli should look into the files before uploading and at least verify they have the correct root-level XML node. Alternatively, a check against a good schema would be even better.

P.S. I've found some dead code in place you changed @joseph-sentry @ https://github.com/codecov/codecov-cli/blame/main/codecov_cli/services/upload/file_finder.py#L217-L219 — it's probably unrelated but contributes to confusion when reading it.

cc @rohan-at-sentry — there might be a deeper issue here, not sure if I should open an issue in the feedback repo..

webknjaz commented 1 month ago

UPD: it seems like this issue has nothing to do with GH statuses not showing up. It still persists after I started enumerating the files explicitly.

rohan-at-sentry commented 1 month ago

@webknjaz thanks for opening it.

For the meta question - Opening it here is fine because you have specific cli based feedback.

Status checks not showing up could potentially be un-related. I see you opened https://github.com/codecov/feedback/issues/511 so I'm happy to continue that convo there.

WRT Test result files being uploaded as coverage, we'll take a look

webknjaz commented 1 month ago

@joseph-sentry @rohan-at-sentry the problem regressed, it seems, and I'm observing requirements.txt (constraints really in that context) being attached unconditionally, in addition to the explicit path I'm passing.

https://github.com/ansible/awx-plugins/actions/runs/11182442655/job/31088790330#step:22:60 lists 10 files included in the upload, only 1 of them being a coverage file passed explicitly via files: .tox/.tmp/.test-results/pytest-3.11/cobertura.xml. The action calls codecov do-upload -Z -f .tox/.tmp/.test-results/pytest-3.11/cobertura.xml -F CI-GHA -F pytest -F OS-macOS -F VM-macos-14 -F Py-3.11.9 --git-service github, which selects the file I wanted and passed explicitly, but then goes ahead and adds 9 more files that have nothing to do with any reports (neither test nor coverage):

info - 2024-10-04 15:14:11,252 -- Found 10 coverage files to report
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp313-darwin-arm64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/.tox/.tmp/.test-results/pytest-3.11/cobertura.xml
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp311-darwin-x86_64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp312-darwin-arm64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp313-darwin-x86_64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp311-darwin-arm64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp313-linux-x86_64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp312-linux-x86_64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp312-darwin-x86_64.txt
info - 2024-10-04 15:14:11,252 -- > /Users/runner/work/awx-plugins/awx-plugins/dependencies/lock-files/coverage-docs-cp311-linux-x86_64.txt

What's that about?

webknjaz commented 1 month ago

Using disable_search: true turned out to be an unobvious workaround.