Skip to content
4 changes: 4 additions & 0 deletions .testcoverage.example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ exclude:
- \.pb\.go$ # excludes all protobuf generated files
- ^pkg/bar # exclude package `pkg/bar`

# (optional; default false)
# When true, requires all coverage-ignore annotations to include explanatory comments
force-annotation-comment: false

# If specified, saves the current test coverage breakdown to this file.
#
# Typically, this breakdown is generated only for main (base) branches and
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ exclude:
- \.pb\.go$ # excludes all protobuf generated files
- ^pkg/bar # exclude package `pkg/bar`

# (optional; default false)
# When true, requires all coverage-ignore annotations to include explanatory comments
force-annotation-comment: false

# If specified, saves the current test coverage breakdown to this file.
#
# Typically, this breakdown is generated only for main (base) branches and
Expand Down
23 changes: 15 additions & 8 deletions pkg/testcoverage/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ func reportForHuman(w io.Writer, result AnalyzeResult) string {

func GenerateCoverageStats(cfg Config) ([]coverage.Stats, error) {
return coverage.GenerateCoverageStats(coverage.Config{ //nolint:wrapcheck // err wrapped above
Profiles: strings.Split(cfg.Profile, ","),
ExcludePaths: cfg.Exclude.Paths,
SourceDir: cfg.SourceDir,
Profiles: strings.Split(cfg.Profile, ","),
ExcludePaths: cfg.Exclude.Paths,
SourceDir: cfg.SourceDir,
ForceAnnotationComment: cfg.ForceAnnotationComment,
})
}

Expand All @@ -103,6 +104,11 @@ func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult {
overrideRules := compileOverridePathRules(cfg)
hasFileOverrides, hasPackageOverrides := detectOverrides(cfg.Override)

var filesWithMissingExplanations []coverage.Stats
if cfg.ForceAnnotationComment {
filesWithMissingExplanations = coverage.StatsFilterWithMissingExplanations(current)
}

return AnalyzeResult{
Threshold: thr,
DiffThreshold: cfg.Diff.Threshold,
Expand All @@ -112,11 +118,12 @@ func Analyze(cfg Config, current, base []coverage.Stats) AnalyzeResult {
PackagesBelowThreshold: checkCoverageStatsBelowThreshold(
makePackageStats(current), thr.Package, overrideRules,
),
FilesWithUncoveredLines: coverage.StatsFilterWithUncoveredLines(current),
TotalStats: coverage.StatsCalcTotal(current),
HasBaseBreakdown: len(base) > 0,
Diff: calculateStatsDiff(current, base),
DiffPercentage: TotalPercentageDiff(current, base),
FilesWithUncoveredLines: coverage.StatsFilterWithUncoveredLines(current),
FilesWithMissingExplanations: filesWithMissingExplanations,
TotalStats: coverage.StatsCalcTotal(current),
HasBaseBreakdown: len(base) > 0,
Diff: calculateStatsDiff(current, base),
DiffPercentage: TotalPercentageDiff(current, base),
}
}

Expand Down
64 changes: 64 additions & 0 deletions pkg/testcoverage/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,40 @@ func TestCheck(t *testing.T) {
assert.Error(t, err)
assert.Contains(t, err.Error(), "failed to load base coverage breakdown")
})

t.Run("valid profile - fail when missing explanations", func(t *testing.T) {
t.Parallel()

buf := &bytes.Buffer{}
cfg := Config{
Profile: profileOK,
SourceDir: sourceDir,
ForceAnnotationComment: true,
}
pass, err := Check(buf, cfg)
assert.False(t, pass)
assert.NoError(t, err)

// Should report missing explanations
assert.Contains(t, buf.String(), "Files with missing explanations for coverage-ignore")
})

t.Run("valid profile - pass when not checking for explanations", func(t *testing.T) {
t.Parallel()

buf := &bytes.Buffer{}
cfg := Config{
Profile: profileOK,
SourceDir: sourceDir,
ForceAnnotationComment: false,
}
pass, err := Check(buf, cfg)
assert.True(t, pass)
assert.NoError(t, err)

// Should not report missing explanations
assert.NotContains(t, buf.String(), "Files with missing explanations for coverage-ignore")
})
}

func TestCheckDiff(t *testing.T) {
Expand Down Expand Up @@ -612,6 +646,36 @@ func Test_Analyze(t *testing.T) {
assert.True(t, result.MeetsDiffThreshold())
assert.Equal(t, 0.01, result.DiffPercentage) //nolint:testifylint //relax
})

t.Run("missing explanations for coverage-ignore", func(t *testing.T) {
t.Parallel()

// Create a stat with missing explanations
stats := []coverage.Stats{
{
Name: prefix + "/foo.go",
Total: 100,
Covered: 100,
AnnotationsWithoutComments: []int{10},
},
}

// When explanations are not required, the check should pass
cfg := Config{
ForceAnnotationComment: false,
}
result := Analyze(cfg, stats, nil)
assert.True(t, result.Pass())
assert.Empty(t, result.FilesWithMissingExplanations)

// When explanations are required, the check should fail
cfg = Config{
ForceAnnotationComment: true,
}
result = Analyze(cfg, stats, nil)
assert.False(t, result.Pass())
assert.NotEmpty(t, result.FilesWithMissingExplanations)
})
}

func TestLoadBaseCoverageBreakdown(t *testing.T) {
Expand Down
23 changes: 12 additions & 11 deletions pkg/testcoverage/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,18 @@ var (
)

type Config struct {
Profile string `yaml:"profile"`
Debug bool `yaml:"-"`
LocalPrefixDeprecated string `yaml:"-"`
SourceDir string `yaml:"-"`
Threshold Threshold `yaml:"threshold"`
Override []Override `yaml:"override,omitempty"`
Exclude Exclude `yaml:"exclude"`
BreakdownFileName string `yaml:"breakdown-file-name"`
GithubActionOutput bool `yaml:"github-action-output"`
Diff Diff `yaml:"diff"`
Badge Badge `yaml:"-"`
Profile string `yaml:"profile"`
Debug bool `yaml:"-"`
LocalPrefixDeprecated string `yaml:"-"`
SourceDir string `yaml:"-"`
Threshold Threshold `yaml:"threshold"`
Override []Override `yaml:"override,omitempty"`
Exclude Exclude `yaml:"exclude"`
BreakdownFileName string `yaml:"breakdown-file-name"`
GithubActionOutput bool `yaml:"github-action-output"`
Diff Diff `yaml:"diff"`
Badge Badge `yaml:"-"`
ForceAnnotationComment bool `yaml:"force-annotation-comment"`
}

type Threshold struct {
Expand Down
4 changes: 3 additions & 1 deletion pkg/testcoverage/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ func nonZeroConfig() Config {
BaseBreakdownFileName: "breakdown.testcoverage",
Threshold: ptr(-1.01),
},
GithubActionOutput: true,
GithubActionOutput: true,
ForceAnnotationComment: false,
}
}

Expand All @@ -265,6 +266,7 @@ threshold:
override:
- threshold: 99
path: pathToFile
force-annotation-comment: false
exclude:
paths:
- path1
Expand Down
50 changes: 38 additions & 12 deletions pkg/testcoverage/coverage/cover.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import (
const IgnoreText = "coverage-ignore"

type Config struct {
Profiles []string
ExcludePaths []string
SourceDir string
Profiles []string
ExcludePaths []string
SourceDir string
ForceAnnotationComment bool
}

func GenerateCoverageStats(cfg Config) ([]Stats, error) {
Expand Down Expand Up @@ -52,7 +53,7 @@ func GenerateCoverageStats(cfg Config) ([]Stats, error) {
continue // this file is excluded
}

s, err := coverageForFile(profile, fi)
s, err := coverageForFile(profile, fi, cfg.ForceAnnotationComment)
if err != nil {
return nil, err
}
Expand All @@ -78,7 +79,7 @@ func GenerateCoverageStats(cfg Config) ([]Stats, error) {
return fileStats, nil
}

func coverageForFile(profile *cover.Profile, fi fileInfo) (Stats, error) {
func coverageForFile(profile *cover.Profile, fi fileInfo, forceComment bool) (Stats, error) {
source, err := os.ReadFile(fi.path)
if err != nil { // coverage-ignore
return Stats{}, fmt.Errorf("failed reading file source [%s]: %w", fi.path, err)
Expand All @@ -89,14 +90,19 @@ func coverageForFile(profile *cover.Profile, fi fileInfo) (Stats, error) {
return Stats{}, err
}

annotations, err := findAnnotations(source)
annotations, withoutComment, err := findAnnotations(source, forceComment)
if err != nil { // coverage-ignore
return Stats{}, err
}

s := sumCoverage(profile, funcs, blocks, annotations)
s.Name = fi.name

s.AnnotationsWithoutComments = make([]int, len(withoutComment))
for i, extent := range withoutComment {
s.AnnotationsWithoutComments[i] = extent.StartLine
}

return s, nil
}

Expand Down Expand Up @@ -251,23 +257,43 @@ func findFilePathMatchingSearch(files *[]fileInfo, search string) string {
return path
}

func findAnnotations(source []byte) ([]extent, error) {
// findAnnotations finds coverage-ignore annotations and checks for explanations
func findAnnotations(source []byte, forceComment bool) ([]extent, []extent, error) {
fset := token.NewFileSet()

node, err := parser.ParseFile(fset, "", source, parser.ParseComments)
if err != nil {
return nil, fmt.Errorf("can't parse comments: %w", err)
return nil, nil, fmt.Errorf("can't parse comments: %w", err)
}

var res []extent
var validAnnotations []extent

var annotationsWithoutComment []extent

for _, c := range node.Comments {
if strings.Contains(c.Text(), IgnoreText) {
res = append(res, newExtent(fset, c))
if !strings.Contains(c.Text(), IgnoreText) {
continue // does not have annotation continue to next comment
}

if forceComment {
if hasComment(c.Text()) {
validAnnotations = append(validAnnotations, newExtent(fset, c))
} else {
annotationsWithoutComment = append(annotationsWithoutComment, newExtent(fset, c))
}
} else {
validAnnotations = append(validAnnotations, newExtent(fset, c))
}
}

return res, nil
return validAnnotations, annotationsWithoutComment, nil
}

// hasComment checks if the coverage-ignore annotation has an explanation comment
func hasComment(text string) bool {
// coverage-ignore should be followed by additional text to be considered an explanation
trimmedComment := strings.TrimSpace(text)
return len(trimmedComment) > len(IgnoreText)
}

func findFuncsAndBlocks(source []byte) ([]extent, []extent, error) {
Expand Down
41 changes: 38 additions & 3 deletions pkg/testcoverage/coverage/cover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ func Test_findFile(t *testing.T) {
func Test_findAnnotations(t *testing.T) {
t.Parallel()

_, err := FindAnnotations(nil)
_, _, err := FindAnnotations(nil, false)
assert.Error(t, err)

_, err = FindAnnotations([]byte{})
_, _, err = FindAnnotations([]byte{}, false)
assert.Error(t, err)

const source = `
Expand All @@ -138,11 +138,46 @@ func Test_findAnnotations(t *testing.T) {
}
`

comments, err := FindAnnotations([]byte(source))
comments, _, err := FindAnnotations([]byte(source), false)
assert.NoError(t, err)
assert.Equal(t, []int{3, 5}, pluckStartLine(comments))
}

func Test_findAnnotationsWithComment(t *testing.T) {
t.Parallel()

_, _, err := FindAnnotations(nil, true)
assert.Error(t, err)

_, _, err = FindAnnotations([]byte{}, true)
assert.Error(t, err)

const source = `
package foo
func foo() int { // coverage-ignore
a := 0
// This is a regular comment
for i := range 10 { // coverage-ignore - this has an explanation
a += i
}
if a > 5 { // coverage-ignore
a = 5
}
return a
}
`

validAnnotations, withoutComment, err := FindAnnotations([]byte(source), true)
assert.NoError(t, err)
assert.Equal(t, []int{6}, pluckStartLine(validAnnotations))
assert.Equal(t, []int{3, 9}, pluckStartLine(withoutComment))

validAnnotations, withoutComment, err = FindAnnotations([]byte(source), false)
assert.NoError(t, err)
assert.Equal(t, []int{3, 6, 9}, pluckStartLine(validAnnotations))
assert.Empty(t, withoutComment)
}

func Test_findFuncs(t *testing.T) {
t.Parallel()

Expand Down
18 changes: 13 additions & 5 deletions pkg/testcoverage/coverage/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import (
)

type Stats struct {
Name string
Total int64
Covered int64
Threshold int
UncoveredLines []int
Name string
Total int64
Covered int64
Threshold int
UncoveredLines []int
AnnotationsWithoutComments []int
}

func (s Stats) UncoveredLinesCount() int {
Expand Down Expand Up @@ -149,6 +150,13 @@ func StatsFilterWithCoveredLines(stats []Stats) []Stats {
})
}

// StatsFilterWithMissingExplanations returns stats that have missing explanations
func StatsFilterWithMissingExplanations(stats []Stats) []Stats {
return filter(stats, func(s Stats) bool {
return len(s.AnnotationsWithoutComments) > 0
})
}

func filter[T any](slice []T, predicate func(T) bool) []T {
var result []T

Expand Down
Loading