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

Clarify when no coverage files appear #2787

Open
codecovdesign opened this issue Oct 29, 2024 · 4 comments
Open

Clarify when no coverage files appear #2787

codecovdesign opened this issue Oct 29, 2024 · 4 comments
Assignees
Labels
bug Something isn't working Feature Request Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months

Comments

@codecovdesign
Copy link
Contributor

codecovdesign commented Oct 29, 2024

*marking as bug to discuss whether bug for fr in sync

Problem to solve

The Codecov report appears in the PR comment even when there are no coverable files – this causes some user confusion as reported recently. (scenario when yaml or JSON file appears)

Image

Solution

IF no coverable files are changed, the report can appear as:

Image

@codecovdesign codecovdesign added the bug Something isn't working label Oct 29, 2024
@thomasrockhu-codecov
Copy link
Contributor

I think this is a feature request @codecovdesign

@eliatcodecov
Copy link

My hunch is this is really straightforward, but is still a feature request. @adrian-codecov can you spend an hour on this and if it's easy knock it out. If not, we can kick it back into the queue.

@drazisil-codecov drazisil-codecov added the Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months label Nov 7, 2024
@adrian-codecov
Copy link

My thoughts is to leverage the diff_totals https://github.com/codecov/worker/blob/6cb0a30e90fcbfa64931d28ae51152306161bade/services/notification/notifiers/mixins/message/writers.py#L31-L32 and seeing if it has any file -> did an experiment with a PR just with a readme change, with a coverable file, and a readme + a coverable file, and only the readme change came with 0 files in it. This should be a reasonable assumption that those files didn't lead to coverage.

Adding that logic unexpectedly broke many tests, I think because those aren't correctly mocking the diff_totals/apply_diff functionality. Notifications tend to be spaghetti code/difficult to deal with, so this could maybe like ~1-2 days to figure out those tests. Probably for the better but a little more time expensive than anticipated.

Please let me know how I should go about this and I'll get it going 👌

@codecovdesign
Copy link
Contributor Author

sync with @eliatcodecov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Feature Request Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months
Projects
None yet
Development

No branches or pull requests

5 participants