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

[Feature Request] Limit report to files which are in the diff #11

Open
MikeDombo opened this issue Apr 16, 2020 · 9 comments
Open

[Feature Request] Limit report to files which are in the diff #11

MikeDombo opened this issue Apr 16, 2020 · 9 comments

Comments

@MikeDombo
Copy link

Thanks for your very helpful action. This is a feature request to limit the reporting comment to only files which were changed in the given PR. I think this should be doable if you check that the file path in the cobertura report is a substring of the file path of a file in the diff.

This would make the report much more useful, especially for larger projects, as it will help reviewers narrow down which coverage report lines are important.

@hannseman
Copy link
Contributor

Thanks!

Have you tried setting only_changed_files: true?

@MikeDombo
Copy link
Author

Oh! Obviously you had thought of that.

Thanks

@MikeDombo
Copy link
Author

Actually, I tried it, but the report has no files in it, despite the diff containing changed tests and files which those tests covered.

@MikeDombo MikeDombo reopened this Apr 17, 2020
@Phil-Barber
Copy link

Phil-Barber commented Apr 6, 2021

Hey I noticed from a little digging that my issue here was that my coverage report was generated with source = <> config set.
This means that the filesChanged list from the report is missing the base dir from the filename property.
eg.
File is treepoints/auth/api/users.py, but in the report it is just auth/api/users.py.
Phil-Barber#1

One possible solution would be to parse the source(s) from the report xml and then explode the filepaths with the added source(s) (and maybe check to see if they exist in the repo?) so that the filtering here works correctly:
https://github.com/5monkeys/cobertura-action/blob/master/src/action.js#L85

I'd be happy to attempt a fix if you're happy with my proposal?

This also may have nothing to do with Mike's problem....

For now I've changed my coverage::run::source to use ::omit instead of source, so the report is generated from the project root

@yuvgeek
Copy link

yuvgeek commented Dec 22, 2021

Any update on this issue?

@scottrice10
Copy link

Bump, if the file names returned from Github don't match with the relative file names in the covertura.xml report, this is still broken. Would be great to fix.

@kevinvalk
Copy link

Same here, you already have link_missing_lines_source_dir. Maybe this can be used as wel in the name matching?

@AbdelHamdi
Copy link

Is this fixed? because I am facing the same issue. The comment does not include the changed files, but a single line for all files report. @Phil-Barber could you please explain more your workaround :)

@Phil-Barber
Copy link

Is this fixed? because I am facing the same issue. The comment does not include the changed files, but a single line for all files report. @Phil-Barber could you please explain more your workaround :)

It's been a while but if I recall correctly:
Using the source configuration in coverage changes the outputted filepaths to be relative to that source directory rather than to the root directory. This created a mismatch between the paths the action is checking (with the source dir prepended) and the paths in the coverage (which don't prepend the source dir).

My workaround was to use the omit configuration instead of the source option to omit everything except the source dir. This means the generated coverage report generates paths that are relative to the root dir and so this action correctly identifies them.

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

No branches or pull requests

7 participants