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

reports are added to output only if thresholds are specified #140

Closed
subham-bdwaj opened this issue Jan 20, 2025 · 3 comments · Fixed by #141
Closed

reports are added to output only if thresholds are specified #140

subham-bdwaj opened this issue Jan 20, 2025 · 3 comments · Fixed by #141

Comments

@subham-bdwaj
Copy link
Contributor

Before releasing this PR - #111

Coverage reports were reported without mentioning project wide file, pkg thresholds. We had workflow setup with the previous functionality.

profile: cover.out
local-prefix: "github.com/owner/service"

threshold:
  file: 0
  package: 0
  total: 10

override:
  - threshold: 90
    path: ^internal/pkg/third_party$
  - threshold: 75
    path: pkg/file.go

This used to report to us when these specific overrides failed.

But after the above mentioned PR was merged, our reports only had total coverage reports since we had 0 thresholds for project wide file and package thresholds. Our workflows started failing and we couldn't understand why from the reports since they were not reported.

Can we keep another config in the .yml file to switch between the two implementations as needed?

Reason: Its not possible in some older projects/code to have overall pkg and file thresholds and adding override = 0 for each of the files becomes tough to manage and takes away a lot of flexibility.

@vladopajic
Copy link
Owner

hey,

when i implemented #111 i had in mind that there might be the case to have 0 thresholds with overrides, like in your example, but i really didn't think that people would use that.

in order to report when there is override with zero thresholds we would need to improve these if statements added in #111:

if thr.Package > 0 || hasPackageOverride() { // Package threshold report
   ... report
}

Can we keep another config in the .yml file to switch between the two implementations as needed?

you mean different version of go-test-coverage to be executed based on configuration?

@vladopajic
Copy link
Owner

Reason: Its not possible in some older projects/code to have overall pkg and file thresholds and adding override = 0 for each of the files becomes tough to manage and takes away a lot of flexibility.

in this case, those projects should invert the configuration:

  • set threshold (for example package threshold) to desired value, then
  • override all packages to it's coverage where threshold coverage is not meet

it's can be bit annoying to set overrides for all those packages, but that's price to pay

p.s. this could be applied to your case as well.

@subhambhardwaj
Copy link
Contributor

in order to report when there is override with zero thresholds we would need to improve these if statements added in #111:

I made these changes in this PR - #141
Please let me know if this works

This will report with zero thresholds but overrides are present

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 a pull request may close this issue.

3 participants