Add more tests

internal/reporter/github_internal_test.go

@@ -0,0 +1,111 @@
+package reporter
+
+import (
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/neilotoole/slogt"
+	"github.com/stretchr/testify/require"
+)
+
+func TestGithubReporterDelete(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	slog.SetDefault(slogt.New(t))
+	r, err := NewGithubReporter(
+		t.Context(),
+		"v0.0.0",
+		srv.URL,
+		srv.URL,
+		time.Second,
+		"token",
+		"owner",
+		"repo",
+		123,
+		50,
+		"HEAD",
+		false,
+	)
+	require.NoError(t, err)
+
+	// Delete should always return nil
+	err = r.Delete(t.Context(), nil, ExistingComment{})
+	require.NoError(t, err)
+}
+
+func TestGithubReporterIsEqual(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer srv.Close()
+
+	slog.SetDefault(slogt.New(t))
+	r, err := NewGithubReporter(
+		t.Context(),
+		"v0.0.0",
+		srv.URL,
+		srv.URL,
+		time.Second,
+		"token",
+		"owner",
+		"repo",
+		123,
+		50,
+		"HEAD",
+		false,
+	)
+	require.NoError(t, err)
+
+	type testCaseT struct {
+		name     string
+		existing ExistingComment
+		pending  PendingComment
+		expected bool
+	}
+
+	testCases := []testCaseT{
+		{
+			name: "different paths",
+			existing: ExistingComment{
+				path: "file1.yml",
+				line: 10,
+				text: "comment",
+			},
+			pending: PendingComment{
+				path: "file2.yml",
+				line: 10,
+				text: "comment",
+			},
+			expected: false,
+		},
+		{
+			name: "same path, different text",
+			existing: ExistingComment{
+				path: "file.yml",
+				line: 10,
+				text: "comment1",
+			},
+			pending: PendingComment{
+				path: "file.yml",
+				line: 10,
+				text: "comment2",
+			},
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Use empty ghPR for simple path/text comparison tests
+			dst := ghPR{files: nil}
+			result := r.IsEqual(dst, tc.existing, tc.pending)
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}

internal/reporter/gitlab.go

@@ -276,7 +276,7 @@ func (gl *GitLabReporter) getUserID(ctx context.Context) (int, error) {
 
 func (gl *GitLabReporter) getMRs(ctx context.Context) (ids []int, err error) {
 	slog.LogAttrs(ctx, slog.LevelDebug, "Finding merge requests for current branch", slog.String("branch", gl.branch))
-	mrs, _, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.BasicMergeRequest, *gitlab.Response, error) {
+	mrs, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.BasicMergeRequest, *gitlab.Response, error) {
 		reqCtx, cancel := context.WithTimeout(ctx, gl.timeout)
 		defer cancel()
 		return gl.client.MergeRequests.ListProjectMergeRequests(gl.project, &gitlab.ListProjectMergeRequestsOptions{
@@ -296,7 +296,7 @@ func (gl *GitLabReporter) getMRs(ctx context.Context) (ids []int, err error) {
 
 func (gl *GitLabReporter) getDiffs(ctx context.Context, mrNum int) ([]*gitlab.MergeRequestDiff, error) {
 	slog.LogAttrs(ctx, slog.LevelDebug, "Getting the list of merge request diffs", slog.Int("mr", mrNum))
-	diffs, _, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.MergeRequestDiff, *gitlab.Response, error) {
+	diffs, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.MergeRequestDiff, *gitlab.Response, error) {
 		reqCtx, cancel := context.WithTimeout(ctx, gl.timeout)
 		defer cancel()
 		return gl.client.MergeRequests.ListMergeRequestDiffs(gl.project, mrNum, &gitlab.ListMergeRequestDiffsOptions{
@@ -308,7 +308,7 @@ func (gl *GitLabReporter) getDiffs(ctx context.Context, mrNum int) ([]*gitlab.Me
 
 func (gl *GitLabReporter) getVersions(ctx context.Context, mrNum int) (*gitlab.MergeRequestDiffVersion, error) {
 	slog.LogAttrs(ctx, slog.LevelDebug, "Getting the list of merge request versions", slog.Int("mr", mrNum))
-	vers, _, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.MergeRequestDiffVersion, *gitlab.Response, error) {
+	vers, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.MergeRequestDiffVersion, *gitlab.Response, error) {
 		reqCtx, cancel := context.WithTimeout(ctx, gl.timeout)
 		defer cancel()
 		return gl.client.MergeRequests.GetMergeRequestDiffVersions(gl.project, mrNum, &gitlab.GetMergeRequestDiffVersionsOptions{
@@ -326,7 +326,7 @@ func (gl *GitLabReporter) getVersions(ctx context.Context, mrNum int) (*gitlab.M
 
 func (gl *GitLabReporter) getDiscussions(ctx context.Context, mrNum int) ([]*gitlab.Discussion, error) {
 	slog.LogAttrs(ctx, slog.LevelDebug, "Getting the list of merge request discussions", slog.Int("mr", mrNum))
-	discs, _, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.Discussion, *gitlab.Response, error) {
+	discs, err := getGitLabPaginated(func(pageNum int) ([]*gitlab.Discussion, *gitlab.Response, error) {
 		reqCtx, cancel := context.WithTimeout(ctx, gl.timeout)
 		defer cancel()
 		return gl.client.Discussions.ListMergeRequestDiscussions(gl.project, mrNum, &gitlab.ListMergeRequestDiscussionsOptions{
@@ -579,21 +579,21 @@ func parseDiffLines(diff string) (lines []diffLine) {
 	return lines
 }
 
-func getGitLabPaginated[T any](searchFunc func(pageNum int) ([]T, *gitlab.Response, error)) ([]T, *gitlab.Response, error) {
+func getGitLabPaginated[T any](searchFunc func(pageNum int) ([]T, *gitlab.Response, error)) ([]T, error) {
 	items := []T{}
 	pageNum := 1
 	for {
 		tempItems, response, err := searchFunc(pageNum)
 		if err != nil {
-			return nil, response, err
+			return nil, err
 		}
 		items = append(items, tempItems...)
 		if response.NextPage == 0 {
 			break
 		}
 		pageNum = response.NextPage
 	}
-	return items, nil, nil
+	return items, nil
 }
 
 func loggifyDiscussion(opt *gitlab.CreateMergeRequestDiscussionOptions) (attrs []slog.Attr) {

internal/reporter/gitlab_logger_test.go

@@ -0,0 +1,38 @@
+package reporter
+
+import (
+	"log/slog"
+	"testing"
+
+	"github.com/neilotoole/slogt"
+)
+
+func TestGitlabLogger(t *testing.T) {
+	t.Run("Info logs message", func(t *testing.T) {
+		slog.SetDefault(slogt.New(t))
+		logger := gitlabLogger{}
+		// This should not panic and should log
+		logger.Info("test info message", "key1", "value1", "key2", 123)
+	})
+
+	t.Run("Warn logs message", func(t *testing.T) {
+		slog.SetDefault(slogt.New(t))
+		logger := gitlabLogger{}
+		// This should not panic and should log
+		logger.Warn("test warn message", "key1", "value1")
+	})
+
+	t.Run("Error logs message", func(t *testing.T) {
+		slog.SetDefault(slogt.New(t))
+		logger := gitlabLogger{}
+		// This should not panic and should log
+		logger.Error("test error message")
+	})
+
+	t.Run("Debug logs message", func(t *testing.T) {
+		slog.SetDefault(slogt.New(t))
+		logger := gitlabLogger{}
+		// This should not panic and should log
+		logger.Debug("test debug message", "key", "value")
+	})
+}

internal/reporter/json_test.go

@@ -0,0 +1,178 @@
+package reporter_test
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/prometheus/common/model"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cloudflare/pint/internal/checks"
+	"github.com/cloudflare/pint/internal/diags"
+	"github.com/cloudflare/pint/internal/discovery"
+	"github.com/cloudflare/pint/internal/parser"
+	"github.com/cloudflare/pint/internal/reporter"
+)
+
+func TestJSONReporter(t *testing.T) {
+	t.Run("NewJSONReporter creates reporter", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		jr := reporter.NewJSONReporter(buf)
+		require.NotNil(t, jr)
+	})
+
+	t.Run("Submit with empty summary", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		jr := reporter.NewJSONReporter(buf)
+
+		summary := reporter.NewSummary([]reporter.Report{})
+		err := jr.Submit(context.Background(), summary)
+		require.NoError(t, err)
+
+		var result []reporter.JSONReport
+		err = json.Unmarshal(buf.Bytes(), &result)
+		require.NoError(t, err)
+		require.Empty(t, result)
+	})
+
+	t.Run("Submit with single report", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		jr := reporter.NewJSONReporter(buf)
+
+		p := parser.NewParser(false, parser.PrometheusSchema, model.UTF8Validation)
+		mockFile := p.Parse(strings.NewReader(`
+- record: test
+  expr: up == 0
+`))
+
+		summary := reporter.NewSummary([]reporter.Report{
+			{
+				Path: discovery.Path{
+					Name:          "test.yml",
+					SymlinkTarget: "test.yml",
+				},
+				Owner:         "team-a",
+				ModifiedLines: []int{2},
+				Rule:          mockFile.Groups[0].Rules[0],
+				Problem: checks.Problem{
+					Lines: diags.LineRange{
+						First: 1,
+						Last:  2,
+					},
+					Reporter: "test-reporter",
+					Summary:  "test error",
+					Details:  "test details",
+					Severity: checks.Fatal,
+				},
+			},
+		})
+
+		err := jr.Submit(context.Background(), summary)
+		require.NoError(t, err)
+
+		var result []reporter.JSONReport
+		err = json.Unmarshal(buf.Bytes(), &result)
+		require.NoError(t, err)
+		require.Len(t, result, 1)
+		require.Equal(t, "test.yml", result[0].Path)
+		require.Equal(t, "team-a", result[0].Owner)
+		require.Equal(t, "test-reporter", result[0].Reporter)
+		require.Equal(t, "test error", result[0].Problem)
+		require.Equal(t, "test details", result[0].Details)
+		require.Equal(t, "Fatal", result[0].Severity)
+		require.Equal(t, []int{1, 2}, result[0].Lines)
+	})
+
+	t.Run("Submit with multiple reports", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		jr := reporter.NewJSONReporter(buf)
+
+		p := parser.NewParser(false, parser.PrometheusSchema, model.UTF8Validation)
+		mockFile := p.Parse(strings.NewReader(`
+- record: test1
+  expr: up == 0
+- record: test2
+  expr: down == 1
+`))
+
+		summary := reporter.NewSummary([]reporter.Report{
+			{
+				Path: discovery.Path{
+					Name:          "test1.yml",
+					SymlinkTarget: "test1.yml",
+				},
+				Rule: mockFile.Groups[0].Rules[0],
+				Problem: checks.Problem{
+					Lines:    diags.LineRange{First: 1, Last: 1},
+					Reporter: "reporter1",
+					Summary:  "error1",
+					Severity: checks.Warning,
+				},
+			},
+			{
+				Path: discovery.Path{
+					Name:          "test2.yml",
+					SymlinkTarget: "test2.yml",
+				},
+				Rule: mockFile.Groups[0].Rules[1],
+				Problem: checks.Problem{
+					Lines:    diags.LineRange{First: 3, Last: 5},
+					Reporter: "reporter2",
+					Summary:  "error2",
+					Details:  "details2",
+					Severity: checks.Bug,
+				},
+			},
+		})
+
+		err := jr.Submit(context.Background(), summary)
+		require.NoError(t, err)
+
+		var result []reporter.JSONReport
+		err = json.Unmarshal(buf.Bytes(), &result)
+		require.NoError(t, err)
+		require.Len(t, result, 2)
+		require.Equal(t, "test1.yml", result[0].Path)
+		require.Equal(t, "test2.yml", result[1].Path)
+	})
+
+	t.Run("Submit with report without owner and details", func(t *testing.T) {
+		buf := &bytes.Buffer{}
+		jr := reporter.NewJSONReporter(buf)
+
+		p := parser.NewParser(false, parser.PrometheusSchema, model.UTF8Validation)
+		mockFile := p.Parse(strings.NewReader(`
+- record: test
+  expr: up
+`))
+
+		summary := reporter.NewSummary([]reporter.Report{
+			{
+				Path: discovery.Path{
+					Name:          "test.yml",
+					SymlinkTarget: "test.yml",
+				},
+				Rule: mockFile.Groups[0].Rules[0],
+				Problem: checks.Problem{
+					Lines:    diags.LineRange{First: 1, Last: 1},
+					Reporter: "test",
+					Summary:  "test",
+					Severity: checks.Information,
+				},
+			},
+		})
+
+		err := jr.Submit(context.Background(), summary)
+		require.NoError(t, err)
+
+		var result []reporter.JSONReport
+		err = json.Unmarshal(buf.Bytes(), &result)
+		require.NoError(t, err)
+		require.Len(t, result, 1)
+		require.Empty(t, result[0].Owner)
+		require.Empty(t, result[0].Details)
+	})
+}