codacy / codacy-coverage-reporter-action

GitHub Action for the codacy-coverage-reporter
Other
58 stars 15 forks source link

[CY-5878] Wildcard in coverage-reports #59

Closed bmulholland closed 1 year ago

bmulholland commented 2 years ago

I'm running parallel tests so I will upload multiple coverage reports. Ideally, I can add a wildcard (or similar) so that I don't have to hardcode each individual filename (one for each thread). That way, we can change the number of threads without updating the list of coverage-reports.

Specifically, right now I have to write: coverage-reports: "coverage/lcov/project-1.lcov,coverage/lcov/project-2.lcov". I'd like a way to replace the list with a general match for any number.

Docs for the raw coverage script have suggestions for using find, which doesn't appear to be supported here either. That would work too.

github-actions[bot] commented 2 years ago

Internal ticket created : CY-5878

stefanvacareanu7 commented 2 years ago

Hi there,

Thank you for reaching out to our community!

The Engineering Team is going to look into better alternatives for uploading multiple coverage reports.

The first solution that crossed my mind is using the pattern: "coverage-reports: 'coverage/lcov/project-*.lcov'", but I imagine this isn't working.

Feel free to share any progress on this matter.

Best, Stefan

bmulholland commented 2 years ago

Yeah a * would be one of my preferred solutions :)

stefanvacareanu7 commented 2 years ago

Hi there,

I created a new dynamic command to send coverage to Codacy:

bash <(curl -Ls https://coverage.codacy.com/get.sh) report \
    -l Java $(echo -r) $(echo $(find coverage_*.xml) | sed 's/ / -r /g')

You can replace find coverage_*xml with any other command that's displaying all coverage reports you want to send.

Does this work for you?

Best, Stefan

bmulholland commented 2 years ago

It's not a drop-in replacement; I'll have to spend 30+ minutes to migrate to the bash command from codacy-coverage-reporter-action. I can try that out sometime, but I don't have time today.

veimox commented 1 year ago

@stefanvacareanu7 This is now supported on the root repo with after https://github.com/codacy/codacy-coverage-reporter/pull/444 thanks to @lolgab. Could we make use of this to close this issue? ❤️

lolgab commented 1 year ago

@veimox Good point :) It should already work out of the box since we download the latest version of the binary when we run the action. Did you try to pass a glob pattern to the action?

veimox commented 1 year ago

@lolgab Kind of... we need something like this to make use of it https://github.com/codacy/codacy-coverage-reporter-action/pull/71

lolgab commented 1 year ago

Consider that we want to keep backward compatibility, so pipelines having a comma separated list of files should work as before. Nevertheless, we could improve the documentation by saying that we support a comma-separated list of files or globs.

veimox commented 1 year ago

Personally I think is actually good that we break the backwards compatibility and we version this on v1/v2 because the one breaking the compatibility is the action with the CLI and that should be outmost respected as one is a reflection of the other. Otherwise they will diverge with time and it's maintenance will be more difficult.

That said, is your project so feel free to apply it how you see better for your team either with that PR or a new one on your end 😊

lolgab commented 1 year ago

@veimox Thank you for the PR and the reminder!

Do you have an example of a glob that doesn't work with the current comma-separated list?

veimox commented 1 year ago

Your welcome!

In the PR example you can find a glob that contains comma's. They are used to avoid repeating common paths (which coincidentally is the use case of the current feature)

lolgab commented 1 year ago

In the PR example you can find a glob that contains comma's. They are used to avoid repeating common paths (which coincidentally is the use case of the current feature)

It makes sense now! I didn't read your PR description carefully. Sorry. Yes, to have proper support for globs we need to avoid using comma-separated values. Maybe for now we can start by doing that behind a flag and somehow deprecate that by logging a warning when this feature is used and propose to use the glob version so that eventually we can switch the default.