From 5e8035d9f8fb9c0f574f53b379c1db0475d24e6a Mon Sep 17 00:00:00 2001 From: Subham Bhardwaj Date: Thu, 23 Jan 2025 16:46:27 +0530 Subject: [PATCH 1/4] report when overall threshold present (or) overrides are present --- pkg/testcoverage/check.go | 3 ++- pkg/testcoverage/check_test.go | 4 ++-- pkg/testcoverage/report.go | 4 ++-- pkg/testcoverage/types.go | 1 + pkg/testcoverage/utils.go | 6 +++--- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/testcoverage/check.go b/pkg/testcoverage/check.go index d5f337be..218d0071 100644 --- a/pkg/testcoverage/check.go +++ b/pkg/testcoverage/check.go @@ -75,10 +75,11 @@ func GenerateCoverageStats(cfg Config) ([]coverage.Stats, error) { func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { thr := cfg.Threshold - overrideRules := compileOverridePathRules(cfg) + overrideRules, hasOverrides := compileOverridePathRules(cfg) return AnalyzeResult{ Threshold: thr, + HasOverrides: hasOverrides, FilesBelowThreshold: checkCoverageStatsBelowThreshold(current, thr.File, overrideRules), PackagesBelowThreshold: checkCoverageStatsBelowThreshold( makePackageStats(current), thr.Package, overrideRules, diff --git a/pkg/testcoverage/check_test.go b/pkg/testcoverage/check_test.go index b421f3bc..8f4c9e47 100644 --- a/pkg/testcoverage/check_test.go +++ b/pkg/testcoverage/check_test.go @@ -118,7 +118,7 @@ func TestCheck(t *testing.T) { pass := Check(buf, cfg) assert.True(t, pass) assertGithubActionErrorsCount(t, buf.String(), 0) - assertHumanReport(t, buf.String(), 1, 0) + assertHumanReport(t, buf.String(), 2, 0) assert.GreaterOrEqual(t, strings.Count(buf.String(), prefix), 0) }) @@ -134,7 +134,7 @@ func TestCheck(t *testing.T) { pass := Check(buf, cfg) assert.False(t, pass) assertGithubActionErrorsCount(t, buf.String(), 0) - assertHumanReport(t, buf.String(), 0, 1) + assertHumanReport(t, buf.String(), 0, 2) assert.GreaterOrEqual(t, strings.Count(buf.String(), prefix), 0) }) diff --git a/pkg/testcoverage/report.go b/pkg/testcoverage/report.go index e6fac393..f1d55994 100644 --- a/pkg/testcoverage/report.go +++ b/pkg/testcoverage/report.go @@ -36,14 +36,14 @@ func reportCoverage(w io.Writer, result AnalyzeResult) { thr := result.Threshold - if thr.File > 0 { // File threshold report + if thr.File > 0 || result.HasOverrides { // File threshold report fmt.Fprintf(tabber, "File coverage threshold (%d%%) satisfied:\t", thr.File) fmt.Fprint(tabber, statusStr(len(result.FilesBelowThreshold) == 0)) reportIssuesForHuman(tabber, result.FilesBelowThreshold) fmt.Fprint(tabber, "\n") } - if thr.Package > 0 { // Package threshold report + if thr.Package > 0 || result.HasOverrides { // Package threshold report fmt.Fprintf(tabber, "Package coverage threshold (%d%%) satisfied:\t", thr.Package) fmt.Fprint(tabber, statusStr(len(result.PackagesBelowThreshold) == 0)) reportIssuesForHuman(tabber, result.PackagesBelowThreshold) diff --git a/pkg/testcoverage/types.go b/pkg/testcoverage/types.go index e853cdf6..17855929 100644 --- a/pkg/testcoverage/types.go +++ b/pkg/testcoverage/types.go @@ -15,6 +15,7 @@ type AnalyzeResult struct { TotalStats coverage.Stats HasBaseBreakdown bool Diff []FileCoverageDiff + HasOverrides bool } func (r *AnalyzeResult) Pass() bool { diff --git a/pkg/testcoverage/utils.go b/pkg/testcoverage/utils.go index 79f4f3a4..d39f4af0 100644 --- a/pkg/testcoverage/utils.go +++ b/pkg/testcoverage/utils.go @@ -19,9 +19,9 @@ func matches(regexps []regRule, str string) (int, bool) { return 0, false } -func compileOverridePathRules(cfg Config) []regRule { +func compileOverridePathRules(cfg Config) ([]regRule, bool) { if len(cfg.Override) == 0 { - return nil + return nil, false } compiled := make([]regRule, len(cfg.Override)) @@ -33,5 +33,5 @@ func compileOverridePathRules(cfg Config) []regRule { } } - return compiled + return compiled, true } From 60426fa6671a7cbcaec52b7373f8aa7a01a7454c Mon Sep 17 00:00:00 2001 From: Subham Bhardwaj Date: Fri, 24 Jan 2025 09:41:49 +0530 Subject: [PATCH 2/4] Add separate checks for file and pkg overrides --- pkg/testcoverage/check.go | 25 +++++++++++++++++++++++-- pkg/testcoverage/report.go | 4 ++-- pkg/testcoverage/types.go | 3 ++- pkg/testcoverage/utils.go | 6 +++--- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/pkg/testcoverage/check.go b/pkg/testcoverage/check.go index 218d0071..838626b6 100644 --- a/pkg/testcoverage/check.go +++ b/pkg/testcoverage/check.go @@ -74,12 +74,18 @@ func GenerateCoverageStats(cfg Config) ([]coverage.Stats, error) { } func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { + var hasFileOverrides, hasPackageOverrides bool thr := cfg.Threshold - overrideRules, hasOverrides := compileOverridePathRules(cfg) + overrideRules := compileOverridePathRules(cfg) + + if len(cfg.Override) > 0 { + hasFileOverrides, hasPackageOverrides = detectOverrides(cfg.Override) + } return AnalyzeResult{ Threshold: thr, - HasOverrides: hasOverrides, + HasFileOverrides: hasFileOverrides, + HasPackageOverrides: hasPackageOverrides, FilesBelowThreshold: checkCoverageStatsBelowThreshold(current, thr.File, overrideRules), PackagesBelowThreshold: checkCoverageStatsBelowThreshold( makePackageStats(current), thr.Package, overrideRules, @@ -90,6 +96,21 @@ func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { } } +func detectOverrides(overrides []Override) (hasFileOverrides, hasPackageOverrides bool) { + for _, override := range overrides { + if strings.HasSuffix(override.Path, ".go") { + hasFileOverrides = true + } + if strings.HasPrefix(override.Path, "^") { + hasPackageOverrides = true + } + if hasFileOverrides && hasPackageOverrides { + return // Early return if both found + } + } + return +} + func saveCoverageBreakdown(cfg Config, stats []coverage.Stats) error { if cfg.BreakdownFileName == "" { return nil diff --git a/pkg/testcoverage/report.go b/pkg/testcoverage/report.go index f1d55994..fc879ed7 100644 --- a/pkg/testcoverage/report.go +++ b/pkg/testcoverage/report.go @@ -36,14 +36,14 @@ func reportCoverage(w io.Writer, result AnalyzeResult) { thr := result.Threshold - if thr.File > 0 || result.HasOverrides { // File threshold report + if thr.File > 0 || result.HasFileOverrides { // File threshold report fmt.Fprintf(tabber, "File coverage threshold (%d%%) satisfied:\t", thr.File) fmt.Fprint(tabber, statusStr(len(result.FilesBelowThreshold) == 0)) reportIssuesForHuman(tabber, result.FilesBelowThreshold) fmt.Fprint(tabber, "\n") } - if thr.Package > 0 || result.HasOverrides { // Package threshold report + if thr.Package > 0 || result.HasPackageOverrides { // Package threshold report fmt.Fprintf(tabber, "Package coverage threshold (%d%%) satisfied:\t", thr.Package) fmt.Fprint(tabber, statusStr(len(result.PackagesBelowThreshold) == 0)) reportIssuesForHuman(tabber, result.PackagesBelowThreshold) diff --git a/pkg/testcoverage/types.go b/pkg/testcoverage/types.go index 17855929..b142648a 100644 --- a/pkg/testcoverage/types.go +++ b/pkg/testcoverage/types.go @@ -15,7 +15,8 @@ type AnalyzeResult struct { TotalStats coverage.Stats HasBaseBreakdown bool Diff []FileCoverageDiff - HasOverrides bool + HasFileOverrides bool + HasPackageOverrides bool } func (r *AnalyzeResult) Pass() bool { diff --git a/pkg/testcoverage/utils.go b/pkg/testcoverage/utils.go index d39f4af0..79f4f3a4 100644 --- a/pkg/testcoverage/utils.go +++ b/pkg/testcoverage/utils.go @@ -19,9 +19,9 @@ func matches(regexps []regRule, str string) (int, bool) { return 0, false } -func compileOverridePathRules(cfg Config) ([]regRule, bool) { +func compileOverridePathRules(cfg Config) []regRule { if len(cfg.Override) == 0 { - return nil, false + return nil } compiled := make([]regRule, len(cfg.Override)) @@ -33,5 +33,5 @@ func compileOverridePathRules(cfg Config) ([]regRule, bool) { } } - return compiled, true + return compiled } From aa2be837aab2f6f9f959ecfdf8be4ec56a4c8b47 Mon Sep 17 00:00:00 2001 From: Subham Bhardwaj Date: Fri, 24 Jan 2025 09:46:15 +0530 Subject: [PATCH 3/4] linting fixes --- pkg/testcoverage/check.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/testcoverage/check.go b/pkg/testcoverage/check.go index 838626b6..9665e047 100644 --- a/pkg/testcoverage/check.go +++ b/pkg/testcoverage/check.go @@ -75,6 +75,7 @@ func GenerateCoverageStats(cfg Config) ([]coverage.Stats, error) { func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { var hasFileOverrides, hasPackageOverrides bool + thr := cfg.Threshold overrideRules := compileOverridePathRules(cfg) @@ -96,19 +97,25 @@ func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { } } -func detectOverrides(overrides []Override) (hasFileOverrides, hasPackageOverrides bool) { +func detectOverrides(overrides []Override) (bool, bool) { + hasFileOverrides := false + hasPackageOverrides := false + for _, override := range overrides { if strings.HasSuffix(override.Path, ".go") { hasFileOverrides = true } + if strings.HasPrefix(override.Path, "^") { hasPackageOverrides = true } + if hasFileOverrides && hasPackageOverrides { - return // Early return if both found + return hasFileOverrides, hasPackageOverrides } } - return + + return hasFileOverrides, hasPackageOverrides } func saveCoverageBreakdown(cfg Config, stats []coverage.Stats) error { From 1313a756f65ed9767b04ed3cc39fa97f566e1aa8 Mon Sep 17 00:00:00 2001 From: Subham Bhardwaj Date: Fri, 24 Jan 2025 17:50:18 +0530 Subject: [PATCH 4/4] check suffix .go and .go$ for file overrides, else its a pkg override --- pkg/testcoverage/check.go | 17 +++-------------- pkg/testcoverage/check_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/pkg/testcoverage/check.go b/pkg/testcoverage/check.go index 9665e047..87a76cf1 100644 --- a/pkg/testcoverage/check.go +++ b/pkg/testcoverage/check.go @@ -74,14 +74,9 @@ func GenerateCoverageStats(cfg Config) ([]coverage.Stats, error) { } func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult { - var hasFileOverrides, hasPackageOverrides bool - thr := cfg.Threshold overrideRules := compileOverridePathRules(cfg) - - if len(cfg.Override) > 0 { - hasFileOverrides, hasPackageOverrides = detectOverrides(cfg.Override) - } + hasFileOverrides, hasPackageOverrides := detectOverrides(cfg.Override) return AnalyzeResult{ Threshold: thr, @@ -102,17 +97,11 @@ func detectOverrides(overrides []Override) (bool, bool) { hasPackageOverrides := false for _, override := range overrides { - if strings.HasSuffix(override.Path, ".go") { + if strings.HasSuffix(override.Path, ".go") || strings.HasSuffix(override.Path, ".go$") { hasFileOverrides = true - } - - if strings.HasPrefix(override.Path, "^") { + } else { hasPackageOverrides = true } - - if hasFileOverrides && hasPackageOverrides { - return hasFileOverrides, hasPackageOverrides - } } return hasFileOverrides, hasPackageOverrides diff --git a/pkg/testcoverage/check_test.go b/pkg/testcoverage/check_test.go index 8f4c9e47..81a12c59 100644 --- a/pkg/testcoverage/check_test.go +++ b/pkg/testcoverage/check_test.go @@ -138,6 +138,38 @@ func TestCheck(t *testing.T) { assert.GreaterOrEqual(t, strings.Count(buf.String(), prefix), 0) }) + t.Run("valid profile - pass after file override", func(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + cfg := Config{ + Profile: profileOK, + Threshold: Threshold{File: 70}, + Override: []Override{{Threshold: 60, Path: "pkg/testcoverage/badgestorer/github.go"}}, + } + pass := Check(buf, cfg) + assert.True(t, pass) + assertGithubActionErrorsCount(t, buf.String(), 0) + assertHumanReport(t, buf.String(), 1, 0) + assert.GreaterOrEqual(t, strings.Count(buf.String(), prefix), 0) + }) + + t.Run("valid profile - fail after file override", func(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + cfg := Config{ + Profile: profileOK, + Threshold: Threshold{File: 70}, + Override: []Override{{Threshold: 80, Path: "pkg/testcoverage/badgestorer/github.go"}}, + } + pass := Check(buf, cfg) + assert.False(t, pass) + assertGithubActionErrorsCount(t, buf.String(), 0) + assertHumanReport(t, buf.String(), 0, 1) + assert.GreaterOrEqual(t, strings.Count(buf.String(), prefix), 0) + }) + t.Run("valid profile - fail couldn't save badge", func(t *testing.T) { t.Parallel()