improvements

Author: vladopajic

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

@@ -262,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))
 		}
 	}
 

pkg/testcoverage/coverage/cover_test.go

@@ -169,7 +169,7 @@ 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, 6, 9}, PluckStartLine(validAnnotations))
 	assert.Equal(t, []int{3, 9}, PluckStartLine(withoutComment))
 
 	validAnnotations, withoutComment, err = FindAnnotations([]byte(source), false)

pkg/testcoverage/report.go

@@ -69,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
 	}
 
@@ -97,11 +97,11 @@ 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
 		}
 
@@ -155,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)
@@ -168,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 {
@@ -189,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)
 		}
 	}
 }

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