chore: follow up improvements after #222

pkg/testcoverage/check_test.go

@@ -278,9 +278,7 @@ func TestCheck(t *testing.T) {
 		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")
+		assert.Contains(t, buf.String(), "Files with missing explanation for coverage-ignore")
 	})
 
 	t.Run("valid profile - pass when not checking for explanations", func(t *testing.T) {
@@ -295,8 +293,6 @@ func TestCheck(t *testing.T) {
 		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")
 	})
 }
@@ -674,6 +670,7 @@ func Test_Analyze(t *testing.T) {
 		}
 		result = Analyze(cfg, stats, nil)
 		assert.False(t, result.Pass())
+		assert.True(t, result.PassCoverage())
 		assert.NotEmpty(t, result.FilesWithMissingExplanations)
 	})
 }

pkg/testcoverage/coverage/cover.go

@@ -97,11 +97,7 @@ func coverageForFile(profile *cover.Profile, fi fileInfo, forceComment bool) (St
 
 	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
-	}
+	s.AnnotationsWithoutComments = pluckStartLine(withoutComment)
 
 	return s, nil
 }
@@ -266,23 +262,17 @@ func findAnnotations(source []byte, forceComment bool) ([]extent, []extent, erro
 		return nil, nil, fmt.Errorf("can't parse comments: %w", err)
 	}
 
-	var validAnnotations []extent
-
-	var annotationsWithoutComment []extent
+	var validAnnotations, annotationsWithoutComment []extent //nolint:prealloc // relax
 
 	for _, c := range node.Comments {
 		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))
+		validAnnotations = append(validAnnotations, newExtent(fset, c))
+
+		if forceComment && !hasComment(c.Text()) {
+			annotationsWithoutComment = append(annotationsWithoutComment, newExtent(fset, c))
 		}
 	}
 
@@ -377,6 +367,15 @@ func hasExtentWithStartLine(ee []extent, startLine int) bool {
 	return found
 }
 
+func pluckStartLine(extents []extent) []int {
+	res := make([]int, len(extents))
+	for i, e := range extents {
+		res[i] = e.StartLine
+	}
+
+	return res
+}
+
 func sumCoverage(profile *cover.Profile, funcs, blocks, annotations []extent) Stats {
 	s := Stats{}
 

pkg/testcoverage/coverage/cover_test.go

@@ -140,7 +140,7 @@ func Test_findAnnotations(t *testing.T) {
 
 	comments, _, err := FindAnnotations([]byte(source), false)
 	assert.NoError(t, err)
-	assert.Equal(t, []int{3, 5}, pluckStartLine(comments))
+	assert.Equal(t, []int{3, 5}, PluckStartLine(comments))
 }
 
 func Test_findAnnotationsWithComment(t *testing.T) {
@@ -169,12 +169,12 @@ func Test_findAnnotationsWithComment(t *testing.T) {
 
 	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))
+	assert.Equal(t, []int{3, 6, 9}, 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.Equal(t, []int{3, 6, 9}, PluckStartLine(validAnnotations))
 	assert.Empty(t, withoutComment)
 }
 
@@ -218,7 +218,7 @@ func Test_findFuncs(t *testing.T) {
 
 	funcs, blocks, err := FindFuncsAndBlocks([]byte(source))
 	assert.NoError(t, err)
-	assert.Equal(t, []int{3, 6, 13, 20}, pluckStartLine(funcs))
+	assert.Equal(t, []int{3, 6, 13, 20}, PluckStartLine(funcs))
 	assert.Equal(t, []Extent{
 		{8, 16, 10, 4},
 		{15, 22, 17, 4},
@@ -262,12 +262,3 @@ func Test_sumCoverage(t *testing.T) {
 	}}
 	assert.Equal(t, expected, s)
 }
-
-func pluckStartLine(extents []Extent) []int {
-	res := make([]int, len(extents))
-	for i, e := range extents {
-		res[i] = e.StartLine
-	}
-
-	return res
-}

pkg/testcoverage/coverage/export_test.go

@@ -7,6 +7,7 @@ var (
 	ParseProfiles      = parseProfiles
 	SumCoverage        = sumCoverage
 	FindGoModFile      = findGoModFile
+	PluckStartLine     = pluckStartLine
 )
 
 type Extent = extent

pkg/testcoverage/report.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"os"
 	"strconv"
+	"strings"
 	"text/tabwriter"
 
 	"github.com/vladopajic/go-test-coverage/v2/pkg/testcoverage/badge"
@@ -68,7 +69,7 @@ func reportIssuesForHuman(w io.Writer, coverageStats []coverage.Stats) {
 }
 
 func reportUncoveredLines(w io.Writer, result AnalyzeResult) {
-	if result.Pass() || len(result.FilesWithUncoveredLines) == 0 {
+	if result.PassCoverage() || len(result.FilesWithUncoveredLines) == 0 {
 		return
 	}
 
@@ -96,21 +97,16 @@ func reportMissingExplanations(w io.Writer, result AnalyzeResult) {
 	tabber := tabwriter.NewWriter(w, 1, 8, 2, '\t', 0) //nolint:mnd // relax
 	defer tabber.Flush()
 
-	fmt.Fprintf(tabber, "\nFiles with missing explanations for coverage-ignore annotations:")
+	fmt.Fprintf(tabber, "\nFiles with missing explanation for coverage-ignore annotation:")
 	fmt.Fprintf(tabber, "\n  file:\tline numbers:")
 
 	for _, stats := range result.FilesWithMissingExplanations {
-		if len(stats.AnnotationsWithoutComments) == 0 {
+		if len(stats.AnnotationsWithoutComments) == 0 { // coverage-ignore
 			continue
 		}
 
-		fmt.Fprintf(tabber, "\n  %s\t", stats.Name)
-
-		separator := ""
-		for _, ann := range stats.AnnotationsWithoutComments {
-			fmt.Fprintf(tabber, "%s%d", separator, ann)
-			separator = ", "
-		}
+		lines := sliceIntsStr(stats.AnnotationsWithoutComments, ", ")
+		fmt.Fprintf(tabber, "\n  %s\t%s", stats.Name, lines)
 	}
 
 	fmt.Fprintf(tabber, "\n")
@@ -159,8 +155,8 @@ func ReportForGithubAction(w io.Writer, result AnalyzeResult) {
 	out := bufio.NewWriter(w)
 	defer out.Flush()
 
-	reportLineError := func(file, title, msg string) {
-		fmt.Fprintf(out, "::error file=%s,title=%s,line=1::%s\n", file, title, msg)
+	reportLineError := func(file, title, msg string, line int) {
+		fmt.Fprintf(out, "::error file=%s,title=%s,line=%d::%s\n", file, title, line, msg)
 	}
 	reportError := func(title, msg string) {
 		fmt.Fprintf(out, "::error title=%s::%s\n", title, msg)
@@ -172,7 +168,7 @@ func ReportForGithubAction(w io.Writer, result AnalyzeResult) {
 			"%s: coverage: %s; threshold: %d%%",
 			title, stats.Str(), stats.Threshold,
 		)
-		reportLineError(stats.Name, title, msg)
+		reportLineError(stats.Name, title, msg, 1)
 	}
 
 	for _, stats := range result.PackagesBelowThreshold {
@@ -193,17 +189,12 @@ func ReportForGithubAction(w io.Writer, result AnalyzeResult) {
 		reportError(title, msg)
 	}
 
-	// Report missing explanations for coverage-ignore annotations
 	for _, stats := range result.FilesWithMissingExplanations {
-		if len(stats.AnnotationsWithoutComments) > 0 {
-			for _, ann := range stats.AnnotationsWithoutComments {
-				title := "Missing explanation for coverage-ignore"
-				msg := title + ": add an explanation after the coverage-ignore annotation"
-
-				file := stats.Name
-				lineNumber := ann
-				fmt.Fprintf(out, "::error file=%s,title=%s,line=%d::%s\n", file, title, lineNumber, msg)
-			}
+		for _, line := range stats.AnnotationsWithoutComments {
+			title := "Missing explanation for coverage-ignore"
+			msg := title + ": add an explanation after the coverage-ignore annotation"
+
+			reportLineError(stats.Name, title, msg, line)
 		}
 	}
 }
@@ -292,3 +283,12 @@ func statusStr(passing bool) string {
 
 	return "FAIL"
 }
+
+func sliceIntsStr(s []int, sep string) string {
+	strs := make([]string, len(s))
+	for i, v := range s {
+		strs[i] = strconv.Itoa(v)
+	}
+
+	return strings.Join(strs, sep)
+}

pkg/testcoverage/report_test.go

@@ -344,31 +344,11 @@ func Test_ReportMissingExplanations(t *testing.T) {
 		ReportForHuman(buf, result)
 
 		output := buf.String()
-		assert.Contains(t, output, "Files with missing explanations for coverage-ignore annotations:")
+		assert.Contains(t, output, "Files with missing explanation for coverage-ignore annotation:")
 		assert.Contains(t, output, "test.go")
 		assert.Contains(t, output, "10, 20")
 	})
 
-	t.Run("with empty annotations list", func(t *testing.T) {
-		t.Parallel()
-
-		buf := &bytes.Buffer{}
-		result := AnalyzeResult{
-			FilesWithMissingExplanations: []coverage.Stats{
-				{
-					Name:                       "test.go",
-					AnnotationsWithoutComments: []int{},
-				},
-			},
-		}
-
-		ReportForHuman(buf, result)
-
-		output := buf.String()
-		// Should not contain the file with empty annotations
-		assert.NotContains(t, output, "test.go\t")
-	})
-
 	t.Run("with no missing explanations", func(t *testing.T) {
 		t.Parallel()
 

pkg/testcoverage/types.go

@@ -32,6 +32,13 @@ func (r *AnalyzeResult) Pass() bool {
 		len(r.FilesWithMissingExplanations) == 0
 }
 
+func (r *AnalyzeResult) PassCoverage() bool {
+	return r.MeetsTotalCoverage() &&
+		len(r.FilesBelowThreshold) == 0 &&
+		len(r.PackagesBelowThreshold) == 0 &&
+		r.MeetsDiffThreshold()
+}
+
 func (r *AnalyzeResult) MeetsDiffThreshold() bool {
 	if r.DiffThreshold == nil || !r.HasBaseBreakdown {
 		return true