diff --git a/.golangci.yml b/.golangci.yml index b455b96..e60204f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,6 +13,9 @@ issues: max-issues-per-linter: 0 # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. max-same-issues: 0 + exclude-rules: + - path: pkg/prx/types/ + text: "var-naming: avoid meaningless package names" formatters: enable: @@ -188,6 +191,8 @@ linters: disabled: true - name: bare-return disabled: true + - name: var-naming + disabled: true # types is a perfectly valid package name rowserrcheck: # database/sql is always checked. diff --git a/README.md b/README.md index c3b3442..dfa76eb 100644 --- a/README.md +++ b/README.md @@ -5,9 +5,9 @@ Go library for fetching pull request data from GitHub, GitLab, and Gitea/Codeber ## Quick Start ```go -import "github.com/codeGROOVE-dev/prx/pkg/prx/fetch" +import "github.com/codeGROOVE-dev/prx/pkg/prx" -data, err := fetch.Fetch(ctx, "https://github.com/golang/go/pull/12345") +data, err := prx.Fetch(ctx, "https://github.com/golang/go/pull/12345") // Works with: GitHub, GitLab, Codeberg, self-hosted instances ``` Auto-detects platform and resolves authentication from `GITHUB_TOKEN`/`GITLAB_TOKEN`/`GITEA_TOKEN` or CLI tools (`gh`, `glab`, `tea`, `berg`). diff --git a/cmd/prx/main.go b/cmd/prx/main.go index 6532297..8db4a91 100644 --- a/cmd/prx/main.go +++ b/cmd/prx/main.go @@ -18,6 +18,7 @@ import ( "github.com/codeGROOVE-dev/prx/pkg/prx/gitea" "github.com/codeGROOVE-dev/prx/pkg/prx/github" "github.com/codeGROOVE-dev/prx/pkg/prx/gitlab" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) var ( @@ -59,7 +60,7 @@ func run() error { prURL := flag.Arg(0) - parsed, err := prx.ParseURL(prURL) + parsed, err := types.ParseURL(prURL) if err != nil { return fmt.Errorf("invalid PR URL: %w", err) } @@ -74,7 +75,7 @@ func run() error { token, err := resolver.Resolve(ctx, platform, parsed.Host) // Authentication is optional for public repos on GitLab/Gitea/Codeberg // Only GitHub strictly requires authentication for most API calls - tokenOptional := parsed.Platform != prx.PlatformGitHub + tokenOptional := parsed.Platform != types.PlatformGitHub if err != nil && !tokenOptional { return fmt.Errorf("authentication failed: %w", err) @@ -89,13 +90,13 @@ func run() error { } // Create platform-specific client - var prxPlatform prx.Platform + var prxPlatform types.Platform switch parsed.Platform { - case prx.PlatformGitHub: + case types.PlatformGitHub: prxPlatform = github.NewPlatform(tokenValue) - case prx.PlatformGitLab: + case types.PlatformGitLab: prxPlatform = gitlab.NewPlatform(tokenValue, gitlab.WithBaseURL("https://"+parsed.Host)) - case prx.PlatformCodeberg: + case types.PlatformCodeberg: prxPlatform = gitea.NewCodebergPlatform(tokenValue) default: // Self-hosted Gitea @@ -108,7 +109,7 @@ func run() error { opts = append(opts, prx.WithLogger(slog.Default())) } if *noCache { - opts = append(opts, prx.WithCacheStore(null.New[string, prx.PullRequestData]())) + opts = append(opts, prx.WithCacheStore(null.New[string, types.PullRequestData]())) } client := prx.NewClientWithPlatform(prxPlatform, opts...) diff --git a/cmd/prx_compare/main.go b/cmd/prx_compare/main.go index 75e2a1a..7f844db 100644 --- a/cmd/prx_compare/main.go +++ b/cmd/prx_compare/main.go @@ -14,6 +14,7 @@ import ( "github.com/codeGROOVE-dev/prx/pkg/prx" "github.com/codeGROOVE-dev/prx/pkg/prx/github" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) const ( @@ -63,7 +64,7 @@ func main() { fmt.Println("\nFull data saved to rest_output.json and graphql_output.json") } -func comparePullRequestData(rest, graphql *prx.PullRequestData) { +func comparePullRequestData(rest, graphql *types.PullRequestData) { // Compare PullRequest fields fmt.Println("=== Pull Request Metadata ===") comparePullRequest(&rest.PullRequest, &graphql.PullRequest) @@ -73,7 +74,7 @@ func comparePullRequestData(rest, graphql *prx.PullRequestData) { compareEvents(rest.Events, graphql.Events) } -func comparePullRequest(rest, graphql *prx.PullRequest) { +func comparePullRequest(rest, graphql *types.PullRequest) { differences, matches := compareFields(rest, graphql) if len(differences) > 0 { @@ -86,7 +87,7 @@ func comparePullRequest(rest, graphql *prx.PullRequest) { fmt.Printf("\nMatching fields: %s\n", strings.Join(matches, ", ")) } -func compareFields(rest, graphql *prx.PullRequest) (differences, matches []string) { +func compareFields(rest, graphql *types.PullRequest) (differences, matches []string) { restVal := reflect.ValueOf(*rest) graphqlVal := reflect.ValueOf(*graphql) restType := restVal.Type() @@ -131,7 +132,7 @@ func comparePointerField(name string, restField, graphqlField reflect.Value) str return "" } -func compareCheckSummary(rest, graphql *prx.PullRequest) { +func compareCheckSummary(rest, graphql *types.PullRequest) { if rest.CheckSummary == nil || graphql.CheckSummary == nil { return } @@ -149,7 +150,7 @@ func compareCheckSummary(rest, graphql *prx.PullRequest) { compareCheckSummaryMaps(rest.CheckSummary, graphql.CheckSummary) } -func compareCheckSummaryMaps(rest, graphql *prx.CheckSummary) { +func compareCheckSummaryMaps(rest, graphql *types.CheckSummary) { compareSummaryMap("Success", rest.Success, graphql.Success) compareSummaryMap("Failing", rest.Failing, graphql.Failing) compareSummaryMap("Pending", rest.Pending, graphql.Pending) @@ -186,7 +187,7 @@ func compareStatusMaps(rest, graphql map[string]string) { } } -func compareEvents(restEvents, graphqlEvents []prx.Event) { +func compareEvents(restEvents, graphqlEvents []types.Event) { // Count events by type restCounts := countEventsByType(restEvents) graphqlCounts := countEventsByType(graphqlEvents) @@ -200,13 +201,13 @@ func compareEvents(restEvents, graphqlEvents []prx.Event) { allTypes[k] = true } - var types []string + var eventTypes []string for t := range allTypes { - types = append(types, t) + eventTypes = append(eventTypes, t) } - sort.Strings(types) + sort.Strings(eventTypes) - for _, eventType := range types { + for _, eventType := range eventTypes { restCount := restCounts[eventType] graphqlCount := graphqlCounts[eventType] if restCount != graphqlCount { @@ -226,7 +227,7 @@ func compareEvents(restEvents, graphqlEvents []prx.Event) { restByType := groupEventsByType(restEvents) graphqlByType := groupEventsByType(graphqlEvents) - for _, eventType := range types { + for _, eventType := range eventTypes { restTypeEvents := restByType[eventType] graphqlTypeEvents := graphqlByType[eventType] @@ -294,7 +295,7 @@ func compareEvents(restEvents, graphqlEvents []prx.Event) { } } -func countEventsByType(events []prx.Event) map[string]int { +func countEventsByType(events []types.Event) map[string]int { counts := make(map[string]int) for i := range events { counts[events[i].Kind]++ @@ -302,15 +303,15 @@ func countEventsByType(events []prx.Event) map[string]int { return counts } -func groupEventsByType(events []prx.Event) map[string][]prx.Event { - grouped := make(map[string][]prx.Event) +func groupEventsByType(events []types.Event) map[string][]types.Event { + grouped := make(map[string][]types.Event) for i := range events { grouped[events[i].Kind] = append(grouped[events[i].Kind], events[i]) } return grouped } -func extractWriteAccess(events []prx.Event) map[string]int { +func extractWriteAccess(events []types.Event) map[string]int { access := make(map[string]int) for i := range events { e := &events[i] @@ -324,7 +325,7 @@ func extractWriteAccess(events []prx.Event) map[string]int { return access } -func extractBots(events []prx.Event) map[string]bool { +func extractBots(events []types.Event) map[string]bool { bots := make(map[string]bool) for i := range events { e := &events[i] diff --git a/go.mod b/go.mod index 7757fd4..bf958ac 100644 --- a/go.mod +++ b/go.mod @@ -7,11 +7,11 @@ require ( github.com/codeGROOVE-dev/fido/pkg/store/localfs v1.10.0 github.com/codeGROOVE-dev/fido/pkg/store/null v1.10.0 github.com/codeGROOVE-dev/retry v1.3.1 + gopkg.in/yaml.v3 v3.0.1 ) require ( github.com/codeGROOVE-dev/fido/pkg/store/compress v1.10.0 // indirect github.com/klauspost/compress v1.18.2 // indirect github.com/puzpuzpuz/xsync/v4 v4.2.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 9685683..cddcae0 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,7 @@ github.com/pierrec/lz4/v4 v4.1.22 h1:cKFw6uJDK+/gfw5BcDL0JL5aBsAFdsIT18eRtLj7VIU github.com/pierrec/lz4/v4 v4.1.22/go.mod h1:gZWDp/Ze/IJXGXf23ltt2EXimqmTUXEy0GFuRQyBid4= github.com/puzpuzpuz/xsync/v4 v4.2.0 h1:dlxm77dZj2c3rxq0/XNvvUKISAmovoXF4a4qM6Wvkr0= github.com/puzpuzpuz/xsync/v4 v4.2.0/go.mod h1:VJDmTCJMBt8igNxnkQd86r+8KUeN1quSfNKu5bLYFQo= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/pkg/prx/client.go b/pkg/prx/client.go index 30e3aad..5c421d3 100644 --- a/pkg/prx/client.go +++ b/pkg/prx/client.go @@ -18,6 +18,7 @@ import ( "github.com/codeGROOVE-dev/fido" "github.com/codeGROOVE-dev/fido/pkg/store/localfs" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) const ( @@ -29,13 +30,13 @@ const ( // PRStore is the interface for PR cache storage backends. // This is an alias for fido.Store with the appropriate type parameters. -type PRStore = fido.Store[string, PullRequestData] +type PRStore = fido.Store[string, types.PullRequestData] // Client provides methods to fetch pull request events from various platforms. type Client struct { - platform Platform + platform types.Platform logger *slog.Logger - prCache *fido.TieredCache[string, PullRequestData] + prCache *fido.TieredCache[string, types.PullRequestData] } // Option is a function that configures a Client. @@ -67,13 +68,13 @@ func WithCacheStore(store PRStore) Option { // For GitHub: NewClientWithPlatform(github.NewPlatform(token), opts...) // For GitLab: NewClientWithPlatform(gitlab.NewPlatform(token), opts...) // For Gitea: NewClientWithPlatform(gitea.NewPlatform(token), opts...) -func NewClient(platform Platform, opts ...Option) *Client { +func NewClient(platform types.Platform, opts ...Option) *Client { return NewClientWithPlatform(platform, opts...) } // NewClientWithPlatform creates a new Client with the given platform. // Use this to create clients for GitLab, Codeberg, or other platforms. -func NewClientWithPlatform(platform Platform, opts ...Option) *Client { +func NewClientWithPlatform(platform types.Platform, opts ...Option) *Client { c := &Client{ platform: platform, logger: slog.Default(), @@ -90,7 +91,7 @@ func NewClientWithPlatform(platform Platform, opts ...Option) *Client { return c } -func createDefaultCache(log *slog.Logger) *fido.TieredCache[string, PullRequestData] { +func createDefaultCache(log *slog.Logger) *fido.TieredCache[string, types.PullRequestData] { dir, err := os.UserCacheDir() if err != nil { dir = os.TempDir() @@ -100,7 +101,7 @@ func createDefaultCache(log *slog.Logger) *fido.TieredCache[string, PullRequestD log.Warn("failed to create cache directory, caching disabled", "error", err) return nil } - store, err := localfs.New[string, PullRequestData]("prx-pr", dir) + store, err := localfs.New[string, types.PullRequestData]("prx-pr", dir) if err != nil { log.Warn("failed to create cache store, caching disabled", "error", err) return nil @@ -114,7 +115,7 @@ func createDefaultCache(log *slog.Logger) *fido.TieredCache[string, PullRequestD } // PullRequest fetches a pull request with all its events and metadata. -func (c *Client) PullRequest(ctx context.Context, owner, repo string, prNumber int) (*PullRequestData, error) { +func (c *Client) PullRequest(ctx context.Context, owner, repo string, prNumber int) (*types.PullRequestData, error) { return c.PullRequestWithReferenceTime(ctx, owner, repo, prNumber, time.Now()) } @@ -124,7 +125,7 @@ func (c *Client) PullRequestWithReferenceTime( owner, repo string, pr int, refTime time.Time, -) (*PullRequestData, error) { +) (*types.PullRequestData, error) { if c.prCache == nil { return c.platform.FetchPR(ctx, owner, repo, pr, refTime) } @@ -150,10 +151,10 @@ func (c *Client) PullRequestWithReferenceTime( "platform", c.platform.Name(), "owner", owner, "repo", repo, "pr", pr) } - result, err := c.prCache.Fetch(ctx, key, func(ctx context.Context) (PullRequestData, error) { + result, err := c.prCache.Fetch(ctx, key, func(ctx context.Context) (types.PullRequestData, error) { data, err := c.platform.FetchPR(ctx, owner, repo, pr, refTime) if err != nil { - return PullRequestData{}, err + return types.PullRequestData{}, err } data.CachedAt = time.Now() return *data, nil @@ -182,7 +183,7 @@ func NewCacheStore(dir string) (PRStore, error) { if err := os.MkdirAll(dir, 0o700); err != nil { return nil, fmt.Errorf("creating cache directory: %w", err) } - store, err := localfs.New[string, PullRequestData]("prx-pr", dir) + store, err := localfs.New[string, types.PullRequestData]("prx-pr", dir) if err != nil { return nil, fmt.Errorf("creating PR cache store: %w", err) } @@ -205,3 +206,15 @@ func collaboratorsCacheKey(owner, repo string) string { func rulesetsCacheKey(owner, repo string) string { return fmt.Sprintf("%s/%s", owner, repo) } + +// isHexString returns true if the string contains only hex characters. +func isHexString(s string) bool { + for i := range s { + c := s[i] + if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') { + continue + } + return false + } + return true +} diff --git a/pkg/prx/client_coverage_test.go b/pkg/prx/client_coverage_test.go new file mode 100644 index 0000000..4a65d59 --- /dev/null +++ b/pkg/prx/client_coverage_test.go @@ -0,0 +1,414 @@ +package prx_test + +import ( + "context" + "errors" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" +) + +// mockPlatform implements types.Platform for testing +type mockPlatform struct { + name string + fetchFunc func(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) +} + +func (m *mockPlatform) Name() string { + return m.name +} + +func (m *mockPlatform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { + if m.fetchFunc != nil { + return m.fetchFunc(ctx, owner, repo, number, refTime) + } + return &types.PullRequestData{ + PullRequest: types.PullRequest{ + Number: number, + Author: "test", + }, + Events: []types.Event{}, + }, nil +} + +// TestNewClient tests the deprecated NewClient function +func TestNewClient(t *testing.T) { + platform := &mockPlatform{name: "test"} + client := prx.NewClient(platform) + if client == nil { + t.Fatal("NewClient returned nil") + } + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + // Verify it works by making a request + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result.PullRequest.Number != 1 { + t.Errorf("Expected PR number 1, got %d", result.PullRequest.Number) + } +} + +// TestNewCacheStore_Errors tests error paths in NewCacheStore +func TestNewCacheStore_Errors(t *testing.T) { + t.Run("relative path error", func(t *testing.T) { + _, err := prx.NewCacheStore("relative/path") + if err == nil { + t.Error("Expected error for relative path") + } + }) + + t.Run("invalid path error", func(t *testing.T) { + // Use a path that cannot be created (e.g., inside a file) + tmpFile := filepath.Join(t.TempDir(), "file.txt") + if err := os.WriteFile(tmpFile, []byte("test"), 0o600); err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + // Try to create a directory inside the file + _, err := prx.NewCacheStore(filepath.Join(tmpFile, "subdir")) + if err == nil { + t.Error("Expected error when creating cache store in invalid path") + } + }) + + t.Run("success case", func(t *testing.T) { + dir := filepath.Join(t.TempDir(), "cache") + store, err := prx.NewCacheStore(dir) + if err != nil { + t.Errorf("NewCacheStore failed: %v", err) + } + if store == nil { + t.Error("NewCacheStore returned nil store") + } + }) +} + +// TestClose_NilCache tests Close with nil cache +func TestClose_NilCache(t *testing.T) { + platform := &mockPlatform{name: "test"} + // Create client without cache store + client := prx.NewClientWithPlatform(platform) + // Force nil cache by not setting one + err := client.Close() + if err != nil { + t.Errorf("Close with nil cache should not error, got: %v", err) + } +} + +// TestPullRequestWithReferenceTime_NilCache tests without cache +func TestPullRequestWithReferenceTime_NilCache(t *testing.T) { + platform := &mockPlatform{ + name: "test", + fetchFunc: func(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { + return &types.PullRequestData{ + PullRequest: types.PullRequest{ + Number: number, + Author: owner, + }, + Events: []types.Event{}, + }, nil + }, + } + + // Create client with no cache (will use nil cache path) + client := prx.NewClientWithPlatform(platform) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + result, err := client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, time.Now()) + if err != nil { + t.Fatalf("PullRequestWithReferenceTime failed: %v", err) + } + if result.PullRequest.Number != 1 { + t.Errorf("Expected PR number 1, got %d", result.PullRequest.Number) + } +} + +// TestPullRequestWithReferenceTime_CacheGetError tests cache get error handling +func TestPullRequestWithReferenceTime_CacheGetError(t *testing.T) { + platform := &mockPlatform{ + name: "test", + fetchFunc: func(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { + return &types.PullRequestData{ + PullRequest: types.PullRequest{ + Number: number, + Author: "test", + }, + Events: []types.Event{}, + CachedAt: time.Now(), + }, nil + }, + } + + cacheDir := t.TempDir() + store, err := prx.NewCacheStore(cacheDir) + if err != nil { + t.Fatalf("NewCacheStore failed: %v", err) + } + + // Create client with cache + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) + client := prx.NewClientWithPlatform(platform, + prx.WithCacheStore(store), + prx.WithLogger(logger), + ) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + + // First call to populate cache + _, err = client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, time.Now()) + if err != nil { + t.Fatalf("First call failed: %v", err) + } + + // Second call should hit cache + result, err := client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, time.Now()) + if err != nil { + t.Fatalf("Second call failed: %v", err) + } + if result.PullRequest.Number != 1 { + t.Errorf("Expected PR number 1, got %d", result.PullRequest.Number) + } +} + +// TestPullRequestWithReferenceTime_FetchError tests fetch error handling +func TestPullRequestWithReferenceTime_FetchError(t *testing.T) { + expectedErr := errors.New("fetch failed") + platform := &mockPlatform{ + name: "test", + fetchFunc: func(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { + return nil, expectedErr + }, + } + + cacheDir := t.TempDir() + store, err := prx.NewCacheStore(cacheDir) + if err != nil { + t.Fatalf("NewCacheStore failed: %v", err) + } + + client := prx.NewClientWithPlatform(platform, prx.WithCacheStore(store)) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + _, err = client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, time.Now()) + if err == nil { + t.Error("Expected error from fetch") + } + if !errors.Is(err, expectedErr) { + t.Errorf("Expected error %v, got %v", expectedErr, err) + } +} + +// TestPullRequestWithReferenceTime_ExpiredCache tests expired cache handling +func TestPullRequestWithReferenceTime_ExpiredCache(t *testing.T) { + callCount := 0 + platform := &mockPlatform{ + name: "test", + fetchFunc: func(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { + callCount++ + return &types.PullRequestData{ + PullRequest: types.PullRequest{ + Number: number, + Author: "test", + }, + Events: []types.Event{}, + CachedAt: time.Now(), + }, nil + }, + } + + cacheDir := t.TempDir() + store, err := prx.NewCacheStore(cacheDir) + if err != nil { + t.Fatalf("NewCacheStore failed: %v", err) + } + + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug})) + client := prx.NewClientWithPlatform(platform, + prx.WithCacheStore(store), + prx.WithLogger(logger), + ) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + + // First call with old reference time + oldRefTime := time.Now().Add(-1 * time.Hour) + _, err = client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, oldRefTime) + if err != nil { + t.Fatalf("First call failed: %v", err) + } + + if callCount != 1 { + t.Errorf("Expected 1 call, got %d", callCount) + } + + // Second call with future reference time should invalidate cache + futureRefTime := time.Now().Add(1 * time.Hour) + _, err = client.PullRequestWithReferenceTime(ctx, "owner", "repo", 1, futureRefTime) + if err != nil { + t.Fatalf("Second call failed: %v", err) + } + + if callCount != 2 { + t.Errorf("Expected 2 calls (cache should be expired), got %d", callCount) + } +} + +// TestWithCacheStore_CreateError tests error handling in WithCacheStore +func TestWithCacheStore_CreateError(t *testing.T) { + // Create a mock store that will trigger an error in fido.NewTiered + // This is tricky because we need a valid store that fido will reject + // For now, just test that the option doesn't panic with a valid store + + cacheDir := t.TempDir() + store, err := prx.NewCacheStore(cacheDir) + if err != nil { + t.Fatalf("NewCacheStore failed: %v", err) + } + + platform := &mockPlatform{name: "test"} + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) + + // This should not panic even if cache creation has issues + client := prx.NewClientWithPlatform(platform, + prx.WithCacheStore(store), + prx.WithLogger(logger), + ) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + // Verify client works + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result == nil { + t.Error("Expected non-nil result") + } +} + +// TestCreateDefaultCache tests the default cache creation logic +func TestCreateDefaultCache(t *testing.T) { + // Test that default cache creation works + platform := &mockPlatform{name: "test"} + + // Create client without explicit cache store - should create default cache + client := prx.NewClientWithPlatform(platform) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result.PullRequest.Number != 1 { + t.Errorf("Expected PR number 1, got %d", result.PullRequest.Number) + } +} + +// TestClientOptions tests various client option combinations +func TestClientOptions(t *testing.T) { + platform := &mockPlatform{name: "test"} + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug})) + + t.Run("with logger only", func(t *testing.T) { + client := prx.NewClientWithPlatform(platform, prx.WithLogger(logger)) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result == nil { + t.Error("Expected non-nil result") + } + }) + + t.Run("with cache and logger", func(t *testing.T) { + cacheDir := t.TempDir() + store, err := prx.NewCacheStore(cacheDir) + if err != nil { + t.Fatalf("NewCacheStore failed: %v", err) + } + + client := prx.NewClientWithPlatform(platform, + prx.WithCacheStore(store), + prx.WithLogger(logger), + ) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result == nil { + t.Error("Expected non-nil result") + } + }) + + t.Run("no options", func(t *testing.T) { + client := prx.NewClientWithPlatform(platform) + defer func() { + if err := client.Close(); err != nil { + t.Errorf("Close failed: %v", err) + } + }() + + ctx := context.Background() + result, err := client.PullRequest(ctx, "owner", "repo", 1) + if err != nil { + t.Fatalf("PullRequest failed: %v", err) + } + if result == nil { + t.Error("Expected non-nil result") + } + }) +} diff --git a/pkg/prx/events.go b/pkg/prx/events.go deleted file mode 100644 index ee4b033..0000000 --- a/pkg/prx/events.go +++ /dev/null @@ -1,106 +0,0 @@ -package prx - -import ( - "time" -) - -// Event kind constants for PR timeline events. -const ( - EventKindCommit = "commit" // EventKindCommit represents a commit event. - EventKindComment = "comment" // EventKindComment represents a comment event. - EventKindReview = "review" // EventKindReview represents a review event. - EventKindReviewComment = "review_comment" // EventKindReviewComment represents a review comment event. - - EventKindLabeled = "labeled" // EventKindLabeled represents a label added event. - EventKindUnlabeled = "unlabeled" // EventKindUnlabeled represents a label removed event. - - EventKindAssigned = "assigned" // EventKindAssigned represents an assignment event. - EventKindUnassigned = "unassigned" // EventKindUnassigned represents an unassignment event. - - EventKindMilestoned = "milestoned" // EventKindMilestoned represents a milestone added event. - EventKindDemilestoned = "demilestoned" // EventKindDemilestoned represents a milestone removed event. - - EventKindReviewRequested = "review_requested" // EventKindReviewRequested represents a review request event. - EventKindReviewRequestRemoved = "review_request_removed" // EventKindReviewRequestRemoved represents a review request removed event. - - EventKindPROpened = "pr_opened" // EventKindPROpened represents a PR opened event. - EventKindPRClosed = "pr_closed" // EventKindPRClosed represents a PR closed event. - EventKindPRMerged = "pr_merged" // EventKindPRMerged represents a PR merge event. - EventKindMerged = "merged" // EventKindMerged represents a merge event from timeline. - EventKindReadyForReview = "ready_for_review" // EventKindReadyForReview represents a ready for review event. - EventKindConvertToDraft = "convert_to_draft" // EventKindConvertToDraft represents a convert to draft event. - EventKindClosed = "closed" // EventKindClosed represents a PR closed event. - EventKindReopened = "reopened" // EventKindReopened represents a PR reopened event. - EventKindRenamedTitle = "renamed_title" // EventKindRenamedTitle represents a title rename event. - - EventKindMentioned = "mentioned" // EventKindMentioned represents a mention event. - EventKindReferenced = "referenced" // EventKindReferenced represents a reference event. - EventKindCrossReferenced = "cross_referenced" // EventKindCrossReferenced represents a cross-reference event. - - EventKindPinned = "pinned" // EventKindPinned represents a pin event. - EventKindUnpinned = "unpinned" // EventKindUnpinned represents an unpin event. - EventKindTransferred = "transferred" // EventKindTransferred represents a transfer event. - - EventKindSubscribed = "subscribed" // EventKindSubscribed represents a subscription event. - EventKindUnsubscribed = "unsubscribed" // EventKindUnsubscribed represents an unsubscription event. - - EventKindHeadRefDeleted = "head_ref_deleted" // EventKindHeadRefDeleted represents a head ref deletion event. - EventKindHeadRefRestored = "head_ref_restored" // EventKindHeadRefRestored represents a head ref restoration event. - EventKindHeadRefForcePushed = "head_ref_force_pushed" // EventKindHeadRefForcePushed represents a head ref force push event. - - EventKindBaseRefChanged = "base_ref_changed" // EventKindBaseRefChanged represents a base ref change event. - EventKindBaseRefForcePushed = "base_ref_force_pushed" // EventKindBaseRefForcePushed represents a base ref force push event. - - EventKindReviewDismissed = "review_dismissed" // EventKindReviewDismissed represents a review dismissed event. - - EventKindLocked = "locked" // EventKindLocked represents a lock event. - EventKindUnlocked = "unlocked" // EventKindUnlocked represents an unlock event. - - EventKindAutoMergeEnabled = "auto_merge_enabled" // EventKindAutoMergeEnabled represents an auto merge enabled event. - EventKindAutoMergeDisabled = "auto_merge_disabled" // EventKindAutoMergeDisabled represents an auto merge disabled event. - EventKindAddedToMergeQueue = "added_to_merge_queue" // EventKindAddedToMergeQueue represents an added to merge queue event. - EventKindRemovedFromMergeQueue = "removed_from_merge_queue" // EventKindRemovedFromMergeQueue represents removal from merge queue. - - // EventKindAutomaticBaseChangeSucceeded represents a successful base change. - EventKindAutomaticBaseChangeSucceeded = "automatic_base_change_succeeded" - // EventKindAutomaticBaseChangeFailed represents a failed base change. - EventKindAutomaticBaseChangeFailed = "automatic_base_change_failed" - - EventKindDeployed = "deployed" // EventKindDeployed represents a deployment event. - // EventKindDeploymentEnvironmentChanged represents a deployment environment change event. - EventKindDeploymentEnvironmentChanged = "deployment_environment_changed" - - EventKindConnected = "connected" // EventKindConnected represents a connected event. - EventKindDisconnected = "disconnected" // EventKindDisconnected represents a disconnected event. - EventKindUserBlocked = "user_blocked" // EventKindUserBlocked represents a user blocked event. - - EventKindStatusCheck = "status_check" // EventKindStatusCheck represents a status check event (from APIs). - EventKindCheckRun = "check_run" // EventKindCheckRun represents a check run event (from APIs). -) - -// WriteAccess constants for the Event.WriteAccess field. -const ( - WriteAccessNo = -2 // User confirmed to not have write access - WriteAccessUnlikely = -1 // User unlikely to have write access (CONTRIBUTOR, NONE, etc.) - WriteAccessNA = 0 // Not applicable/not set (omitted from JSON) - WriteAccessLikely = 1 // User likely has write access but unable to confirm (MEMBER with 403 API response) - WriteAccessDefinitely = 2 // User definitely has write access (OWNER, COLLABORATOR, or confirmed via API) -) - -// Event represents a single event that occurred on a pull request. -// Each event captures who did what and when, with additional context depending on the event type. -type Event struct { - Timestamp time.Time `json:"timestamp"` - Kind string `json:"kind"` - Actor string `json:"actor"` - Target string `json:"target,omitempty"` - Outcome string `json:"outcome,omitempty"` - Body string `json:"body,omitempty"` - Description string `json:"description,omitempty"` - WriteAccess int `json:"write_access,omitempty"` - Bot bool `json:"bot,omitempty"` - TargetIsBot bool `json:"target_is_bot,omitempty"` - Question bool `json:"question,omitempty"` - Required bool `json:"required,omitempty"` - Outdated bool `json:"outdated,omitempty"` // For review comments: indicates comment is on outdated code -} diff --git a/pkg/prx/example_test.go b/pkg/prx/example_test.go index eebe831..c8c186e 100644 --- a/pkg/prx/example_test.go +++ b/pkg/prx/example_test.go @@ -12,7 +12,7 @@ func Example() { // The simplest way: just pass a URL // Authentication is automatically resolved from environment or CLI tools ctx := context.Background() - data, err := fetch.Fetch(ctx, "https://github.com/owner/repo/pull/123") + data, err := fetch.PullRequest(ctx, "https://github.com/owner/repo/pull/123") if err != nil { log.Fatal(err) } @@ -33,39 +33,39 @@ func Example() { } } -func ExampleFetch() { +func ExamplePullRequest() { ctx := context.Background() // Works with GitHub - data, err := fetch.Fetch(ctx, "https://github.com/owner/repo/pull/123") + data, err := fetch.PullRequest(ctx, "https://github.com/owner/repo/pull/123") if err != nil { log.Fatal(err) } fmt.Printf("GitHub PR #%d\n", data.PullRequest.Number) // Works with GitLab - data, err = fetch.Fetch(ctx, "https://gitlab.com/owner/repo/-/merge_requests/456") + data, err = fetch.PullRequest(ctx, "https://gitlab.com/owner/repo/-/merge_requests/456") if err != nil { log.Fatal(err) } fmt.Printf("GitLab MR #%d\n", data.PullRequest.Number) // Works with Codeberg - data, err = fetch.Fetch(ctx, "https://codeberg.org/owner/repo/pulls/789") + data, err = fetch.PullRequest(ctx, "https://codeberg.org/owner/repo/pulls/789") if err != nil { log.Fatal(err) } fmt.Printf("Codeberg PR #%d\n", data.PullRequest.Number) // Works with any Gitea instance - data, err = fetch.Fetch(ctx, "https://gitea.example.com/owner/repo/pulls/100") + data, err = fetch.PullRequest(ctx, "https://gitea.example.com/owner/repo/pulls/100") if err != nil { log.Fatal(err) } fmt.Printf("Gitea PR #%d\n", data.PullRequest.Number) // URL fragments and query parameters are automatically stripped - data, err = fetch.Fetch(ctx, "https://github.com/owner/repo/pull/123?tab=checks#issuecomment-456") + data, err = fetch.PullRequest(ctx, "https://github.com/owner/repo/pull/123?tab=checks#issuecomment-456") if err != nil { log.Fatal(err) } @@ -78,7 +78,7 @@ func ExampleClient_PullRequest() { ctx := context.Background() // Fetch using the simple API - data, err := fetch.Fetch(ctx, "https://github.com/golang/go/pull/123") + data, err := fetch.PullRequest(ctx, "https://github.com/golang/go/pull/123") if err != nil { log.Fatal(err) } diff --git a/pkg/prx/fetch/fetch.go b/pkg/prx/fetch/fetch.go index 2fc6b52..baa64f6 100644 --- a/pkg/prx/fetch/fetch.go +++ b/pkg/prx/fetch/fetch.go @@ -12,17 +12,18 @@ import ( "github.com/codeGROOVE-dev/prx/pkg/prx/gitea" "github.com/codeGROOVE-dev/prx/pkg/prx/github" "github.com/codeGROOVE-dev/prx/pkg/prx/gitlab" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) -// Fetch automatically detects the platform from a PR/MR URL, resolves authentication, +// PullRequest automatically detects the platform from a PR/MR URL, resolves authentication, // and fetches the pull request data. This is the simplest way to use prx. // // Example: // -// data, err := fetch.Fetch(ctx, "https://github.com/owner/repo/pull/123") -// data, err := fetch.Fetch(ctx, "https://gitlab.com/owner/repo/-/merge_requests/456") -// data, err := fetch.Fetch(ctx, "https://codeberg.org/owner/repo/pulls/789") -// data, err := fetch.Fetch(ctx, "https://gitea.example.com/owner/repo/pulls/100") +// data, err := fetch.PullRequest(ctx, "https://github.com/owner/repo/pull/123") +// data, err := fetch.PullRequest(ctx, "https://gitlab.com/owner/repo/-/merge_requests/456") +// data, err := fetch.PullRequest(ctx, "https://codeberg.org/owner/repo/pulls/789") +// data, err := fetch.PullRequest(ctx, "https://gitea.example.com/owner/repo/pulls/100") // // The function will: // - Parse the URL to detect platform, owner, repo, and PR number @@ -39,9 +40,9 @@ import ( // Unknown hosts default to Gitea unless the URL indicates GitHub or GitLab. // // Use WithOptions to pass additional client configuration options. -func Fetch(ctx context.Context, url string, opts ...prx.Option) (*prx.PullRequestData, error) { +func PullRequest(ctx context.Context, url string, opts ...prx.Option) (*types.PullRequestData, error) { // Parse URL to get platform, owner, repo, PR number - parsed, err := prx.ParseURL(url) + parsed, err := types.ParseURL(url) if err != nil { return nil, fmt.Errorf("parsing URL: %w", err) } @@ -55,13 +56,13 @@ func Fetch(ctx context.Context, url string, opts ...prx.Option) (*prx.PullReques } // Create platform-specific client - var platform prx.Platform + var platform types.Platform switch parsed.Platform { - case prx.PlatformGitHub: + case types.PlatformGitHub: platform = github.NewPlatform(token.Value) - case prx.PlatformGitLab: + case types.PlatformGitLab: platform = gitlab.NewPlatform(token.Value, gitlab.WithBaseURL("https://"+parsed.Host)) - case prx.PlatformCodeberg: + case types.PlatformCodeberg: platform = gitea.NewCodebergPlatform(token.Value) default: // Default to Gitea for unknown platforms @@ -78,10 +79,3 @@ func Fetch(ctx context.Context, url string, opts ...prx.Option) (*prx.PullReques return client.PullRequest(ctx, parsed.Owner, parsed.Repo, parsed.Number) } - -// WithOptions is an alias for Fetch for backwards compatibility. -// -// Deprecated: Use Fetch directly, which now accepts options. -func WithOptions(ctx context.Context, url string, opts ...prx.Option) (*prx.PullRequestData, error) { - return Fetch(ctx, url, opts...) -} diff --git a/pkg/prx/fetch/fetch_test.go b/pkg/prx/fetch/fetch_test.go index 2a1ab22..ef05a6e 100644 --- a/pkg/prx/fetch/fetch_test.go +++ b/pkg/prx/fetch/fetch_test.go @@ -7,35 +7,25 @@ import ( "github.com/codeGROOVE-dev/prx/pkg/prx/fetch" ) -func TestFetch_InvalidURL(t *testing.T) { +func TestPullRequest_InvalidURL(t *testing.T) { ctx := context.Background() - _, err := fetch.Fetch(ctx, "not-a-valid-url") + _, err := fetch.PullRequest(ctx, "not-a-valid-url") if err == nil { t.Error("expected error for invalid URL, got nil") } } -func TestFetch_EmptyURL(t *testing.T) { +func TestPullRequest_EmptyURL(t *testing.T) { ctx := context.Background() - _, err := fetch.Fetch(ctx, "") + _, err := fetch.PullRequest(ctx, "") if err == nil { t.Error("expected error for empty URL, got nil") } } -func TestWithOptions_Compatibility(t *testing.T) { - ctx := context.Background() - - // Test the deprecated WithOptions function still works - _, err := fetch.WithOptions(ctx, "invalid-url") - if err == nil { - t.Error("expected error for invalid URL, got nil") - } -} - -// Note: This package is a thin convenience wrapper that combines: +// Note: This is a thin convenience wrapper that combines: // - URL parsing (tested in pkg/prx/url_test.go) // - Authentication resolution (tested in pkg/prx/auth/auth_test.go) // - Platform client creation (tested in each platform package) diff --git a/pkg/prx/gitea/platform.go b/pkg/prx/gitea/platform.go index c637357..d9b36be 100644 --- a/pkg/prx/gitea/platform.go +++ b/pkg/prx/gitea/platform.go @@ -14,7 +14,7 @@ import ( "time" "github.com/codeGROOVE-dev/fido" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // Cache TTL constants. @@ -115,13 +115,13 @@ func NewCodebergPlatform(token string, opts ...Option) *Platform { // Name returns the platform identifier. func (p *Platform) Name() string { if strings.Contains(p.baseURL, "codeberg.org") { - return prx.PlatformCodeberg + return types.PlatformCodeberg } return "gitea" } // FetchPR retrieves a pull request with all events and metadata. -func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*prx.PullRequestData, error) { +func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { p.logger.Info("fetching pull request via Gitea REST API", "owner", owner, "repo", repo, "pr", number) @@ -165,9 +165,9 @@ func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, }) // Finalize with calculated summaries. - prx.FinalizePullRequest(&pullRequest, events, nil, "") + types.FinalizePullRequest(&pullRequest, events, nil, "") - return &prx.PullRequestData{ + return &types.PullRequestData{ CachedAt: time.Now(), PullRequest: pullRequest, Events: events, @@ -439,8 +439,8 @@ func (p *Platform) doRequest(ctx context.Context, url string, result any) (err e // Conversion methods. -func convertPullRequest(pr *pullRequest, reviews []review) prx.PullRequest { - result := prx.PullRequest{ +func convertPullRequest(pr *pullRequest, reviews []review) types.PullRequest { + result := types.PullRequest{ Number: pr.Number, Title: pr.Title, Body: pr.Body, @@ -476,9 +476,9 @@ func convertPullRequest(pr *pullRequest, reviews []review) prx.PullRequest { } // Set reviewers with their states. - result.Reviewers = make(map[string]prx.ReviewState) + result.Reviewers = make(map[string]types.ReviewState) for _, r := range pr.RequestedReviewers { - result.Reviewers[r.Login] = prx.ReviewStatePending + result.Reviewers[r.Login] = types.ReviewStatePending } // Update reviewer states from reviews. @@ -503,16 +503,16 @@ func convertPullRequest(pr *pullRequest, reviews []review) prx.PullRequest { return result } -func convertReviewState(state string) prx.ReviewState { +func convertReviewState(state string) types.ReviewState { switch strings.ToUpper(state) { case "APPROVED": - return prx.ReviewStateApproved + return types.ReviewStateApproved case "REQUEST_CHANGES": - return prx.ReviewStateChangesRequested + return types.ReviewStateChangesRequested case "COMMENT": - return prx.ReviewStateCommented + return types.ReviewStateCommented default: - return prx.ReviewStatePending + return types.ReviewStatePending } } @@ -522,13 +522,13 @@ func convertToEvents( comments []comment, commits []commit, timeline []timelineEvent, -) []prx.Event { - var events []prx.Event +) []types.Event { + var events []types.Event // Add PR opened event. - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: pr.CreatedAt, - Kind: prx.EventKindPROpened, + Kind: types.EventKindPROpened, Actor: pr.User.Login, }) @@ -542,9 +542,9 @@ func convertToEvents( if idx := strings.IndexByte(msg, '\n'); idx >= 0 { msg = msg[:idx] } - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: commits[i].Commit.Author.Date, - Kind: prx.EventKindCommit, + Kind: types.EventKindCommit, Actor: actor, Body: commits[i].SHA[:7], Description: msg, @@ -553,25 +553,25 @@ func convertToEvents( // Add review events. for i := range reviews { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: reviews[i].SubmittedAt, - Kind: prx.EventKindReview, + Kind: types.EventKindReview, Actor: reviews[i].User.Login, Outcome: convertReviewOutcome(reviews[i].State), Body: reviews[i].Body, - Question: prx.ContainsQuestion(reviews[i].Body), + Question: types.ContainsQuestion(reviews[i].Body), Outdated: reviews[i].Stale || reviews[i].Dismissed, }) } // Add comment events. for i := range comments { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: comments[i].CreatedAt, - Kind: prx.EventKindComment, + Kind: types.EventKindComment, Actor: comments[i].User.Login, Body: comments[i].Body, - Question: prx.ContainsQuestion(comments[i].Body), + Question: types.ContainsQuestion(comments[i].Body), }) } @@ -589,15 +589,15 @@ func convertToEvents( if pr.MergedBy != nil { actor = pr.MergedBy.Login } - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *pr.MergedAt, - Kind: prx.EventKindPRMerged, + Kind: types.EventKindPRMerged, Actor: actor, }) } else if pr.ClosedAt != nil && pr.State == "closed" { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *pr.ClosedAt, - Kind: prx.EventKindPRClosed, + Kind: types.EventKindPRClosed, }) } @@ -617,7 +617,7 @@ func convertReviewOutcome(state string) string { } } -func convertTimelineEvent(event *timelineEvent) *prx.Event { +func convertTimelineEvent(event *timelineEvent) *types.Event { actor := "" if event.User != nil { actor = event.User.Login @@ -628,9 +628,9 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Label == nil { return nil } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindLabeled, + Kind: types.EventKindLabeled, Actor: actor, Description: event.Label.Name, } @@ -638,9 +638,9 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Label == nil { return nil } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindUnlabeled, + Kind: types.EventKindUnlabeled, Actor: actor, Description: event.Label.Name, } @@ -648,9 +648,9 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Assignee == nil { return nil } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindAssigned, + Kind: types.EventKindAssigned, Actor: actor, Target: event.Assignee.Login, } @@ -658,9 +658,9 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Assignee == nil { return nil } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindUnassigned, + Kind: types.EventKindUnassigned, Actor: actor, Target: event.Assignee.Login, } @@ -669,9 +669,9 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Assignee != nil { target = event.Assignee.Login } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindReviewRequested, + Kind: types.EventKindReviewRequested, Actor: actor, Target: target, } @@ -680,48 +680,48 @@ func convertTimelineEvent(event *timelineEvent) *prx.Event { if event.Assignee != nil { target = event.Assignee.Login } - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindReviewRequestRemoved, + Kind: types.EventKindReviewRequestRemoved, Actor: actor, Target: target, } case "close": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindClosed, + Kind: types.EventKindClosed, Actor: actor, } case "reopen": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindReopened, + Kind: types.EventKindReopened, Actor: actor, } case "change_title": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindRenamedTitle, + Kind: types.EventKindRenamedTitle, Actor: actor, Description: event.Body, } case "change_ref": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindBaseRefChanged, + Kind: types.EventKindBaseRefChanged, Actor: actor, Description: fmt.Sprintf("%s -> %s", event.OldRef, event.NewRef), } case "merge": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindMerged, + Kind: types.EventKindMerged, Actor: actor, } case "comment_ref", "issue_ref", "pull_ref": - return &prx.Event{ + return &types.Event{ Timestamp: event.CreatedAt, - Kind: prx.EventKindCrossReferenced, + Kind: types.EventKindCrossReferenced, Actor: actor, Description: event.Body, } diff --git a/pkg/prx/gitea/platform_test.go b/pkg/prx/gitea/platform_test.go index f06fdce..6d4c747 100644 --- a/pkg/prx/gitea/platform_test.go +++ b/pkg/prx/gitea/platform_test.go @@ -9,7 +9,7 @@ import ( "testing" "time" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // Test helper function @@ -29,7 +29,7 @@ func TestPlatform_Name(t *testing.T) { { name: "codeberg", baseURL: "https://codeberg.org", - want: prx.PlatformCodeberg, + want: types.PlatformCodeberg, }, { name: "self-hosted gitea", @@ -50,8 +50,8 @@ func TestPlatform_Name(t *testing.T) { func TestNewCodebergPlatform(t *testing.T) { p := NewCodebergPlatform("test-token") - if p.Name() != prx.PlatformCodeberg { - t.Errorf("NewCodebergPlatform().Name() = %q, want %q", p.Name(), prx.PlatformCodeberg) + if p.Name() != types.PlatformCodeberg { + t.Errorf("NewCodebergPlatform().Name() = %q, want %q", p.Name(), types.PlatformCodeberg) } } @@ -227,11 +227,11 @@ func TestPlatform_FetchPR(t *testing.T) { if len(pr.Reviewers) != 2 { t.Errorf("len(Reviewers) = %d, want 2", len(pr.Reviewers)) } - if pr.Reviewers["reviewer1"] != prx.ReviewStatePending { - t.Errorf("Reviewers[reviewer1] = %v, want %v", pr.Reviewers["reviewer1"], prx.ReviewStatePending) + if pr.Reviewers["reviewer1"] != types.ReviewStatePending { + t.Errorf("Reviewers[reviewer1] = %v, want %v", pr.Reviewers["reviewer1"], types.ReviewStatePending) } - if pr.Reviewers["reviewer2"] != prx.ReviewStateApproved { - t.Errorf("Reviewers[reviewer2] = %v, want %v", pr.Reviewers["reviewer2"], prx.ReviewStateApproved) + if pr.Reviewers["reviewer2"] != types.ReviewStateApproved { + t.Errorf("Reviewers[reviewer2] = %v, want %v", pr.Reviewers["reviewer2"], types.ReviewStateApproved) } // Verify events @@ -244,7 +244,7 @@ func TestPlatform_FetchPR(t *testing.T) { for _, e := range data.Events { eventTypes[e.Kind] = true } - expectedTypes := []string{prx.EventKindPROpened, prx.EventKindCommit, prx.EventKindReview, prx.EventKindComment, prx.EventKindLabeled} + expectedTypes := []string{types.EventKindPROpened, types.EventKindCommit, types.EventKindReview, types.EventKindComment, types.EventKindLabeled} for _, et := range expectedTypes { if !eventTypes[et] { t.Errorf("Missing event type %q in events", et) @@ -323,7 +323,7 @@ func TestPlatform_FetchPR_Merged(t *testing.T) { // Check for merged event hasMergedEvent := false for _, e := range data.Events { - if e.Kind == prx.EventKindPRMerged { + if e.Kind == types.EventKindPRMerged { hasMergedEvent = true if e.Actor != "merger" { t.Errorf("Merged event actor = %q, want %q", e.Actor, "merger") @@ -418,18 +418,18 @@ func TestPlatform_FetchPR_APIError(t *testing.T) { func TestConvertReviewState(t *testing.T) { tests := []struct { input string - want prx.ReviewState + want types.ReviewState }{ - {"APPROVED", prx.ReviewStateApproved}, - {"approved", prx.ReviewStateApproved}, - {"REQUEST_CHANGES", prx.ReviewStateChangesRequested}, - {"request_changes", prx.ReviewStateChangesRequested}, - {"COMMENT", prx.ReviewStateCommented}, - {"comment", prx.ReviewStateCommented}, - {"PENDING", prx.ReviewStatePending}, - {"pending", prx.ReviewStatePending}, - {"unknown", prx.ReviewStatePending}, - {"", prx.ReviewStatePending}, + {"APPROVED", types.ReviewStateApproved}, + {"approved", types.ReviewStateApproved}, + {"REQUEST_CHANGES", types.ReviewStateChangesRequested}, + {"request_changes", types.ReviewStateChangesRequested}, + {"COMMENT", types.ReviewStateCommented}, + {"comment", types.ReviewStateCommented}, + {"PENDING", types.ReviewStatePending}, + {"pending", types.ReviewStatePending}, + {"unknown", types.ReviewStatePending}, + {"", types.ReviewStatePending}, } for _, tt := range tests { @@ -481,13 +481,13 @@ func TestConvertTimelineEvent(t *testing.T) { { name: "label event", event: timelineEvent{Type: "label", CreatedAt: now, User: testUser, Label: testLabel}, - wantKind: prx.EventKindLabeled, + wantKind: types.EventKindLabeled, wantActor: "testuser", }, { name: "unlabel event", event: timelineEvent{Type: "unlabel", CreatedAt: now, User: testUser, Label: testLabel}, - wantKind: prx.EventKindUnlabeled, + wantKind: types.EventKindUnlabeled, wantActor: "testuser", }, { @@ -498,55 +498,55 @@ func TestConvertTimelineEvent(t *testing.T) { { name: "assignees event", event: timelineEvent{Type: "assignees", CreatedAt: now, User: testUser, Assignee: testAssignee}, - wantKind: prx.EventKindAssigned, + wantKind: types.EventKindAssigned, wantActor: "testuser", }, { name: "unassignees event", event: timelineEvent{Type: "unassignees", CreatedAt: now, User: testUser, Assignee: testAssignee}, - wantKind: prx.EventKindUnassigned, + wantKind: types.EventKindUnassigned, wantActor: "testuser", }, { name: "review_requested event", event: timelineEvent{Type: "review_requested", CreatedAt: now, User: testUser, Assignee: testAssignee}, - wantKind: prx.EventKindReviewRequested, + wantKind: types.EventKindReviewRequested, wantActor: "testuser", }, { name: "close event", event: timelineEvent{Type: "close", CreatedAt: now, User: testUser}, - wantKind: prx.EventKindClosed, + wantKind: types.EventKindClosed, wantActor: "testuser", }, { name: "reopen event", event: timelineEvent{Type: "reopen", CreatedAt: now, User: testUser}, - wantKind: prx.EventKindReopened, + wantKind: types.EventKindReopened, wantActor: "testuser", }, { name: "change_title event", event: timelineEvent{Type: "change_title", CreatedAt: now, User: testUser, Body: "old -> new"}, - wantKind: prx.EventKindRenamedTitle, + wantKind: types.EventKindRenamedTitle, wantActor: "testuser", }, { name: "change_ref event", event: timelineEvent{Type: "change_ref", CreatedAt: now, User: testUser, OldRef: "old", NewRef: "new"}, - wantKind: prx.EventKindBaseRefChanged, + wantKind: types.EventKindBaseRefChanged, wantActor: "testuser", }, { name: "merge event", event: timelineEvent{Type: "merge", CreatedAt: now, User: testUser}, - wantKind: prx.EventKindMerged, + wantKind: types.EventKindMerged, wantActor: "testuser", }, { name: "comment_ref event", event: timelineEvent{Type: "comment_ref", CreatedAt: now, User: testUser}, - wantKind: prx.EventKindCrossReferenced, + wantKind: types.EventKindCrossReferenced, wantActor: "testuser", }, { @@ -557,7 +557,7 @@ func TestConvertTimelineEvent(t *testing.T) { { name: "event without user", event: timelineEvent{Type: "close", CreatedAt: now}, - wantKind: prx.EventKindClosed, + wantKind: types.EventKindClosed, wantActor: "", }, } @@ -675,10 +675,10 @@ func TestConvertPullRequest(t *testing.T) { if len(result.Assignees) != 2 { t.Errorf("len(Assignees) = %d, want 2", len(result.Assignees)) } - if result.Reviewers["reviewer"] != prx.ReviewStatePending { + if result.Reviewers["reviewer"] != types.ReviewStatePending { t.Errorf("Reviewers[reviewer] = %v, want pending", result.Reviewers["reviewer"]) } - if result.Reviewers["approver"] != prx.ReviewStateApproved { + if result.Reviewers["approver"] != types.ReviewStateApproved { t.Errorf("Reviewers[approver] = %v, want approved", result.Reviewers["approver"]) } if result.MergeableState != "clean" { @@ -712,7 +712,7 @@ func TestConvertPullRequest_StaleReview(t *testing.T) { if _, exists := result.Reviewers["reviewer2"]; exists { t.Error("Dismissed review should not update reviewer state") } - if result.Reviewers["reviewer3"] != prx.ReviewStateApproved { + if result.Reviewers["reviewer3"] != types.ReviewStateApproved { t.Errorf("Reviewers[reviewer3] = %v, want approved", result.Reviewers["reviewer3"]) } } diff --git a/pkg/prx/github/check_run_history_test.go b/pkg/prx/github/check_run_history_test.go index 96fc62c..1e32431 100644 --- a/pkg/prx/github/check_run_history_test.go +++ b/pkg/prx/github/check_run_history_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // TestCheckRunHistory_MultipleCommits tests that we capture check run failures @@ -180,7 +181,7 @@ func TestCheckRunHistory_MultipleCommits(t *testing.T) { checkRunCount := 0 failureCount := 0 successCount := 0 - var checkRunEvents []prx.Event + var checkRunEvents []types.Event for _, event := range prData.Events { if event.Kind == "check_run" { @@ -220,9 +221,9 @@ func TestCheckRunHistory_MultipleCommits(t *testing.T) { } } - // Verify the prx.CheckSummary shows the LATEST state (both checks passing) + // Verify the types.CheckSummary shows the LATEST state (both checks passing) if prData.PullRequest.CheckSummary == nil { - t.Fatal("Expected prx.CheckSummary to be set") + t.Fatal("Expected types.CheckSummary to be set") } if len(prData.PullRequest.CheckSummary.Success) != 2 { @@ -324,7 +325,7 @@ func TestCheckRunHistory_CommitSHAPreservation(t *testing.T) { } // Find the commit event - var commitEvent *prx.Event + var commitEvent *types.Event for i := range prData.Events { if prData.Events[i].Kind == "commit" { commitEvent = &prData.Events[i] @@ -347,7 +348,7 @@ func TestCheckRunHistory_CommitSHAPreservation(t *testing.T) { } // Find the check_run event - var checkRunEvent *prx.Event + var checkRunEvent *types.Event for i := range prData.Events { if prData.Events[i].Kind == "check_run" { checkRunEvent = &prData.Events[i] @@ -370,11 +371,11 @@ func TestCheckRunHistory_CommitSHAPreservation(t *testing.T) { } } -// TestCheckRunHistory_LatestStateCalculation tests that Calculateprx.CheckSummary +// TestCheckRunHistory_LatestStateCalculation tests that Calculatetypes.CheckSummary // correctly identifies the latest state when multiple runs exist for the same check. func TestCheckRunHistory_LatestStateCalculation(t *testing.T) { // Create events with multiple runs of the same check at different times - events := []prx.Event{ + events := []types.Event{ { Kind: "check_run", Timestamp: time.Date(2025, 1, 1, 10, 0, 0, 0, time.UTC), @@ -398,7 +399,7 @@ func TestCheckRunHistory_LatestStateCalculation(t *testing.T) { }, } - summary := prx.CalculateCheckSummary(events, nil) + summary := types.CalculateCheckSummary(events, nil) // The latest run (12:00) was successful, so the check should be in Success if len(summary.Success) != 1 { @@ -418,7 +419,7 @@ func TestCheckRunHistory_LatestStateCalculation(t *testing.T) { // correctly handles events that arrive out of chronological order. func TestCheckRunHistory_OutOfOrderEvents(t *testing.T) { // Events intentionally out of order - older success should not override newer failure - events := []prx.Event{ + events := []types.Event{ { Kind: "check_run", Timestamp: time.Date(2025, 1, 1, 12, 0, 0, 0, time.UTC), // Newest (failure) @@ -442,7 +443,7 @@ func TestCheckRunHistory_OutOfOrderEvents(t *testing.T) { }, } - summary := prx.CalculateCheckSummary(events, nil) + summary := types.CalculateCheckSummary(events, nil) // The latest run (12:00) failed, so the check should be in Failing if len(summary.Failing) != 1 { @@ -464,58 +465,58 @@ func TestCalculateTestStateFromCheckSummary(t *testing.T) { tests := []struct { name string - summary *prx.CheckSummary + summary *types.CheckSummary wantState string }{ { name: "nil summary returns none", summary: nil, - wantState: prx.TestStateNone, + wantState: types.TestStateNone, }, { name: "failing checks returns failing", - summary: &prx.CheckSummary{ + summary: &types.CheckSummary{ Success: map[string]string{"test1": "passed"}, Failing: map[string]string{"test2": "failed"}, Pending: map[string]string{}, }, - wantState: prx.TestStateFailing, + wantState: types.TestStateFailing, }, { name: "only pending checks returns pending", - summary: &prx.CheckSummary{ + summary: &types.CheckSummary{ Success: map[string]string{}, Failing: map[string]string{}, Pending: map[string]string{"test1": "waiting"}, }, - wantState: prx.TestStatePending, + wantState: types.TestStatePending, }, { name: "only successful checks returns passing", - summary: &prx.CheckSummary{ + summary: &types.CheckSummary{ Success: map[string]string{"test1": "passed", "test2": "passed"}, Failing: map[string]string{}, Pending: map[string]string{}, }, - wantState: prx.TestStatePassing, + wantState: types.TestStatePassing, }, { name: "no checks returns none", - summary: &prx.CheckSummary{ + summary: &types.CheckSummary{ Success: map[string]string{}, Failing: map[string]string{}, Pending: map[string]string{}, }, - wantState: prx.TestStateNone, + wantState: types.TestStateNone, }, { name: "failing takes precedence over pending", - summary: &prx.CheckSummary{ + summary: &types.CheckSummary{ Success: map[string]string{}, Failing: map[string]string{"test1": "failed"}, Pending: map[string]string{"test2": "waiting"}, }, - wantState: prx.TestStateFailing, + wantState: types.TestStateFailing, }, } diff --git a/pkg/prx/github/collaborators_test.go b/pkg/prx/github/collaborators_test.go index d726756..f688913 100644 --- a/pkg/prx/github/collaborators_test.go +++ b/pkg/prx/github/collaborators_test.go @@ -6,7 +6,7 @@ import ( "log/slog" "testing" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" "github.com/codeGROOVE-dev/fido" ) @@ -22,16 +22,16 @@ func TestPermissionToWriteAccess(t *testing.T) { permission string expected int }{ - {"admin", prx.WriteAccessDefinitely}, - {"maintain", prx.WriteAccessDefinitely}, - {"write", prx.WriteAccessDefinitely}, - {"read", prx.WriteAccessNo}, - {"triage", prx.WriteAccessNo}, - {"none", prx.WriteAccessNo}, - {"", prx.WriteAccessUnlikely}, // Not in collaborators list - {"unknown", prx.WriteAccessUnlikely}, // Unknown permission - {"ADMIN", prx.WriteAccessUnlikely}, // Case sensitive - not matched - {"something", prx.WriteAccessUnlikely}, // Invalid permission + {"admin", types.WriteAccessDefinitely}, + {"maintain", types.WriteAccessDefinitely}, + {"write", types.WriteAccessDefinitely}, + {"read", types.WriteAccessNo}, + {"triage", types.WriteAccessNo}, + {"none", types.WriteAccessNo}, + {"", types.WriteAccessUnlikely}, // Not in collaborators list + {"unknown", types.WriteAccessUnlikely}, // Unknown permission + {"ADMIN", types.WriteAccessUnlikely}, // Case sensitive - not matched + {"something", types.WriteAccessUnlikely}, // Invalid permission } for _, tt := range tests { @@ -40,11 +40,11 @@ func TestPermissionToWriteAccess(t *testing.T) { var result int switch tt.permission { case "admin", "maintain", "write": - result = prx.WriteAccessDefinitely + result = types.WriteAccessDefinitely case "read", "triage", "none": - result = prx.WriteAccessNo + result = types.WriteAccessNo default: - result = prx.WriteAccessUnlikely + result = types.WriteAccessUnlikely } if result != tt.expected { t.Errorf("permission mapping for %q = %d, want %d", @@ -105,37 +105,37 @@ func TestWriteAccessFromAssociationWithCache(t *testing.T) { name: "member with admin permission", user: "alice", permission: "admin", - expected: prx.WriteAccessDefinitely, + expected: types.WriteAccessDefinitely, }, { name: "member with write permission", user: "bob", permission: "write", - expected: prx.WriteAccessDefinitely, + expected: types.WriteAccessDefinitely, }, { name: "member with maintain permission", user: "charlie", permission: "maintain", - expected: prx.WriteAccessDefinitely, + expected: types.WriteAccessDefinitely, }, { name: "member with read permission", user: "david", permission: "read", - expected: prx.WriteAccessNo, + expected: types.WriteAccessNo, }, { name: "member with triage permission", user: "eve", permission: "triage", - expected: prx.WriteAccessNo, + expected: types.WriteAccessNo, }, { name: "member not in collaborators list", user: "frank", permission: "", // Not in the cache - expected: prx.WriteAccessUnlikely, + expected: types.WriteAccessUnlikely, }, } @@ -197,9 +197,9 @@ func TestWriteAccessFromAssociationCacheHit(t *testing.T) { } result := p.writeAccessFromAssociation(ctx, "codeGROOVE-dev", "goose", "tstromberg", "MEMBER") - if result != prx.WriteAccessDefinitely { + if result != types.WriteAccessDefinitely { t.Errorf("writeAccessFromAssociation(MEMBER, tstromberg) = %d, want %d", - result, prx.WriteAccessDefinitely) + result, types.WriteAccessDefinitely) } } @@ -219,12 +219,12 @@ func TestWriteAccessFromAssociationNonMember(t *testing.T) { association string expected int }{ - {"OWNER", prx.WriteAccessDefinitely}, - {"COLLABORATOR", prx.WriteAccessDefinitely}, - {"CONTRIBUTOR", prx.WriteAccessUnlikely}, - {"NONE", prx.WriteAccessUnlikely}, - {"FIRST_TIME_CONTRIBUTOR", prx.WriteAccessUnlikely}, - {"FIRST_TIMER", prx.WriteAccessUnlikely}, + {"OWNER", types.WriteAccessDefinitely}, + {"COLLABORATOR", types.WriteAccessDefinitely}, + {"CONTRIBUTOR", types.WriteAccessUnlikely}, + {"NONE", types.WriteAccessUnlikely}, + {"FIRST_TIME_CONTRIBUTOR", types.WriteAccessUnlikely}, + {"FIRST_TIMER", types.WriteAccessUnlikely}, } for _, tt := range tests { diff --git a/pkg/prx/github/graphql_complete_test.go b/pkg/prx/github/graphql_complete_test.go index 1511aad..79f111f 100644 --- a/pkg/prx/github/graphql_complete_test.go +++ b/pkg/prx/github/graphql_complete_test.go @@ -9,7 +9,7 @@ import ( "time" "github.com/codeGROOVE-dev/fido" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestIsBot(t *testing.T) { @@ -274,7 +274,7 @@ func TestConvertGraphQLReviewCommentsWithOutdated(t *testing.T) { events := platform.convertGraphQLToEventsComplete(ctx, data, "testowner", "testrepo") // Filter to only review_comment events - var reviewComments []prx.Event + var reviewComments []types.Event for _, event := range events { if event.Kind == "review_comment" { reviewComments = append(reviewComments, event) diff --git a/pkg/prx/github/graphql_parity_test.go b/pkg/prx/github/graphql_parity_test.go index 19a18bc..a1c53b5 100644 --- a/pkg/prx/github/graphql_parity_test.go +++ b/pkg/prx/github/graphql_parity_test.go @@ -10,7 +10,7 @@ import ( "testing" "time" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" "github.com/codeGROOVE-dev/fido" ) @@ -47,7 +47,7 @@ func TestGraphQLParity(t *testing.T) { } // comparePullRequestData compares REST and GraphQL results -func comparePullRequestData(t *testing.T, rest, graphql *prx.PullRequestData) { +func comparePullRequestData(t *testing.T, rest, graphql *types.PullRequestData) { t.Helper() // Compare PullRequest fields pr1 := rest.PullRequest @@ -91,7 +91,7 @@ func comparePullRequestData(t *testing.T, rest, graphql *prx.PullRequestData) { } // countEventsByType counts events by their Kind -func countEventsByType(events []prx.Event) map[string]int { +func countEventsByType(events []types.Event) map[string]int { counts := make(map[string]int) for i := range events { counts[events[i].Kind]++ @@ -100,7 +100,7 @@ func countEventsByType(events []prx.Event) map[string]int { } // compareEvents compares event details -func compareEvents(t *testing.T, restEvents, graphqlEvents []prx.Event) { +func compareEvents(t *testing.T, restEvents, graphqlEvents []types.Event) { t.Helper() // Sort events by timestamp and kind for comparison sort.Slice(restEvents, func(i, j int) bool { @@ -130,7 +130,7 @@ func compareEvents(t *testing.T, restEvents, graphqlEvents []prx.Event) { } // For events with write access, ensure it's preserved - if rest.WriteAccess != prx.WriteAccessNA && graphql.WriteAccess == prx.WriteAccessNA { + if rest.WriteAccess != types.WriteAccessNA && graphql.WriteAccess == types.WriteAccessNA { t.Errorf("WriteAccess lost for event %s by %s: REST=%d, GraphQL=%d", rest.Kind, rest.Actor, rest.WriteAccess, graphql.WriteAccess) } @@ -277,14 +277,14 @@ func TestWriteAccessMapping(t *testing.T) { association string expected int }{ - {"OWNER", prx.WriteAccessDefinitely}, - {"COLLABORATOR", prx.WriteAccessDefinitely}, - {"MEMBER", prx.WriteAccessLikely}, // Falls back to likely when collaborators API unavailable - {"CONTRIBUTOR", prx.WriteAccessUnlikely}, - {"NONE", prx.WriteAccessUnlikely}, - {"FIRST_TIME_CONTRIBUTOR", prx.WriteAccessUnlikely}, - {"FIRST_TIMER", prx.WriteAccessUnlikely}, - {"UNKNOWN", prx.WriteAccessNA}, + {"OWNER", types.WriteAccessDefinitely}, + {"COLLABORATOR", types.WriteAccessDefinitely}, + {"MEMBER", types.WriteAccessLikely}, // Falls back to likely when collaborators API unavailable + {"CONTRIBUTOR", types.WriteAccessUnlikely}, + {"NONE", types.WriteAccessUnlikely}, + {"FIRST_TIME_CONTRIBUTOR", types.WriteAccessUnlikely}, + {"FIRST_TIMER", types.WriteAccessUnlikely}, + {"UNKNOWN", types.WriteAccessNA}, } for _, tt := range tests { diff --git a/pkg/prx/github/platform.go b/pkg/prx/github/platform.go index 35369d3..edbdb6c 100644 --- a/pkg/prx/github/platform.go +++ b/pkg/prx/github/platform.go @@ -11,7 +11,7 @@ import ( "time" "github.com/codeGROOVE-dev/fido" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) const ( @@ -29,7 +29,7 @@ const ( // cachedCheckRuns stores check run events with a timestamp for cache validation. type cachedCheckRuns struct { CachedAt time.Time - Events []prx.Event + Events []types.Event } // Platform implements the prx.Platform interface for GitHub. @@ -118,7 +118,7 @@ func (*Platform) Name() string { } // FetchPR retrieves a pull request with all events and metadata. -func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*prx.PullRequestData, error) { +func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { p.logger.InfoContext(ctx, "fetching pull request via GraphQL", "owner", owner, "repo", repo, "pr", number) prData, err := p.fetchPullRequestCompleteViaGraphQL(ctx, owner, repo, number) @@ -170,7 +170,7 @@ func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, } // fetchPullRequestCompleteViaGraphQL fetches all PR data in a single GraphQL query. -func (p *Platform) fetchPullRequestCompleteViaGraphQL(ctx context.Context, owner, repo string, prNumber int) (*prx.PullRequestData, error) { +func (p *Platform) fetchPullRequestCompleteViaGraphQL(ctx context.Context, owner, repo string, prNumber int) (*types.PullRequestData, error) { data, err := p.executeGraphQL(ctx, owner, repo, prNumber) if err != nil { return nil, err @@ -180,16 +180,16 @@ func (p *Platform) fetchPullRequestCompleteViaGraphQL(ctx context.Context, owner events := p.convertGraphQLToEventsComplete(ctx, data, owner, repo) requiredChecks := p.extractRequiredChecksFromGraphQL(data) - events = prx.FilterEvents(events) + events = types.FilterEvents(events) sort.Slice(events, func(i, j int) bool { return events[i].Timestamp.Before(events[j].Timestamp) }) - prx.UpgradeWriteAccess(events) + types.UpgradeWriteAccess(events) testState := p.calculateTestStateFromGraphQL(data) - prx.FinalizePullRequest(&pr, events, requiredChecks, testState) + types.FinalizePullRequest(&pr, events, requiredChecks, testState) - return &prx.PullRequestData{ + return &types.PullRequestData{ PullRequest: pr, Events: events, }, nil @@ -255,11 +255,11 @@ func (p *Platform) executeGraphQL(ctx context.Context, owner, repo string, prNum } // convertGraphQLToPullRequest converts GraphQL data to PullRequest. -func (p *Platform) convertGraphQLToPullRequest(ctx context.Context, data *graphQLPullRequestComplete, owner, repo string) prx.PullRequest { - pr := prx.PullRequest{ +func (p *Platform) convertGraphQLToPullRequest(ctx context.Context, data *graphQLPullRequestComplete, owner, repo string) types.PullRequest { + pr := types.PullRequest{ Number: data.Number, Title: data.Title, - Body: prx.Truncate(data.Body), + Body: types.Truncate(data.Body), Author: data.Author.Login, State: strings.ToLower(data.State), CreatedAt: data.CreatedAt, @@ -321,24 +321,24 @@ func (p *Platform) convertGraphQLToPullRequest(ctx context.Context, data *graphQ } // convertGraphQLToEventsComplete converts GraphQL data to Events. -func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *graphQLPullRequestComplete, owner, repo string) []prx.Event { - var events []prx.Event +func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *graphQLPullRequestComplete, owner, repo string) []types.Event { + var events []types.Event - events = append(events, prx.Event{ - Kind: prx.EventKindPROpened, + events = append(events, types.Event{ + Kind: types.EventKindPROpened, Timestamp: data.CreatedAt, Actor: data.Author.Login, - Body: prx.Truncate(data.Body), + Body: types.Truncate(data.Body), Bot: isBot(data.Author), WriteAccess: p.writeAccessFromAssociation(ctx, owner, repo, data.Author.Login, data.AuthorAssociation), }) for _, node := range data.Commits.Nodes { - event := prx.Event{ - Kind: prx.EventKindCommit, + event := types.Event{ + Kind: types.EventKindCommit, Timestamp: node.Commit.CommittedDate, Body: node.Commit.OID, - Description: prx.Truncate(node.Commit.Message), + Description: types.Truncate(node.Commit.Message), } if node.Commit.Author.User != nil { event.Actor = node.Commit.Author.User.Login @@ -358,13 +358,13 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra if review.SubmittedAt != nil { timestamp = *review.SubmittedAt } - event := prx.Event{ - Kind: prx.EventKindReview, + event := types.Event{ + Kind: types.EventKindReview, Timestamp: timestamp, Actor: review.Author.Login, - Body: prx.Truncate(review.Body), + Body: types.Truncate(review.Body), Outcome: strings.ToLower(review.State), - Question: prx.ContainsQuestion(review.Body), + Question: types.ContainsQuestion(review.Body), Bot: isBot(review.Author), WriteAccess: p.writeAccessFromAssociation(ctx, owner, repo, review.Author.Login, review.AuthorAssociation), } @@ -375,12 +375,12 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra thread := &data.ReviewThreads.Nodes[i] for j := range thread.Comments.Nodes { comment := &thread.Comments.Nodes[j] - event := prx.Event{ - Kind: prx.EventKindReviewComment, + event := types.Event{ + Kind: types.EventKindReviewComment, Timestamp: comment.CreatedAt, Actor: comment.Author.Login, - Body: prx.Truncate(comment.Body), - Question: prx.ContainsQuestion(comment.Body), + Body: types.Truncate(comment.Body), + Question: types.ContainsQuestion(comment.Body), Bot: isBot(comment.Author), WriteAccess: p.writeAccessFromAssociation(ctx, owner, repo, comment.Author.Login, comment.AuthorAssociation), Outdated: comment.Outdated, @@ -390,12 +390,12 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra } for _, comment := range data.Comments.Nodes { - event := prx.Event{ - Kind: prx.EventKindComment, + event := types.Event{ + Kind: types.EventKindComment, Timestamp: comment.CreatedAt, Actor: comment.Author.Login, - Body: prx.Truncate(comment.Body), - Question: prx.ContainsQuestion(comment.Body), + Body: types.Truncate(comment.Body), + Question: types.ContainsQuestion(comment.Body), Bot: isBot(comment.Author), WriteAccess: p.writeAccessFromAssociation(ctx, owner, repo, comment.Author.Login, comment.AuthorAssociation), } @@ -420,8 +420,8 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra } if node.StartedAt != nil { - events = append(events, prx.Event{ - Kind: prx.EventKindCheckRun, + events = append(events, types.Event{ + Kind: types.EventKindCheckRun, Timestamp: *node.StartedAt, Body: node.Name, Outcome: strings.ToLower(node.Status), @@ -431,8 +431,8 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra } if node.CompletedAt != nil { - events = append(events, prx.Event{ - Kind: prx.EventKindCheckRun, + events = append(events, types.Event{ + Kind: types.EventKindCheckRun, Timestamp: *node.CompletedAt, Body: node.Name, Outcome: strings.ToLower(node.Conclusion), @@ -445,8 +445,8 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra if node.CreatedAt == nil { continue } - event := prx.Event{ - Kind: prx.EventKindStatusCheck, + event := types.Event{ + Kind: types.EventKindStatusCheck, Timestamp: *node.CreatedAt, Outcome: strings.ToLower(node.State), Body: node.Context, @@ -471,13 +471,13 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra } if data.ClosedAt != nil && !data.IsDraft { - event := prx.Event{ - Kind: prx.EventKindPRClosed, + event := types.Event{ + Kind: types.EventKindPRClosed, Timestamp: *data.ClosedAt, } if data.MergedBy != nil { event.Actor = data.MergedBy.Login - event.Kind = prx.EventKindPRMerged + event.Kind = types.EventKindPRMerged event.Bot = isBot(*data.MergedBy) } events = append(events, event) @@ -489,7 +489,7 @@ func (p *Platform) convertGraphQLToEventsComplete(ctx context.Context, data *gra // parseGraphQLTimelineEvent parses a single timeline event. // //nolint:gocognit,maintidx,revive // High complexity justified - must handle all GitHub timeline event types -func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]any, _, _ string) *prx.Event { +func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]any, _, _ string) *types.Event { typename, ok := item["__typename"].(string) if !ok { return nil @@ -535,7 +535,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an return nil } - event := &prx.Event{ + event := &types.Event{ Timestamp: *createdAt, Actor: getActor(), Bot: isActorBot(), @@ -543,7 +543,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an switch typename { case "AssignedEvent": - event.Kind = prx.EventKindAssigned + event.Kind = types.EventKindAssigned if assignee, ok := item["assignee"].(map[string]any); ok { if login, ok := assignee["login"].(string); ok { event.Target = login @@ -551,7 +551,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "UnassignedEvent": - event.Kind = prx.EventKindUnassigned + event.Kind = types.EventKindUnassigned if assignee, ok := item["assignee"].(map[string]any); ok { if login, ok := assignee["login"].(string); ok { event.Target = login @@ -559,7 +559,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "LabeledEvent": - event.Kind = prx.EventKindLabeled + event.Kind = types.EventKindLabeled if label, ok := item["label"].(map[string]any); ok { if name, ok := label["name"].(string); ok { event.Target = name @@ -567,7 +567,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "UnlabeledEvent": - event.Kind = prx.EventKindUnlabeled + event.Kind = types.EventKindUnlabeled if label, ok := item["label"].(map[string]any); ok { if name, ok := label["name"].(string); ok { event.Target = name @@ -575,19 +575,19 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "MilestonedEvent": - event.Kind = prx.EventKindMilestoned + event.Kind = types.EventKindMilestoned if title, ok := item["milestoneTitle"].(string); ok { event.Target = title } case "DemilestonedEvent": - event.Kind = prx.EventKindDemilestoned + event.Kind = types.EventKindDemilestoned if title, ok := item["milestoneTitle"].(string); ok { event.Target = title } case "ReviewRequestedEvent": - event.Kind = prx.EventKindReviewRequested + event.Kind = types.EventKindReviewRequested if reviewer, ok := item["requestedReviewer"].(map[string]any); ok { if login, ok := reviewer["login"].(string); ok { event.Target = login @@ -597,7 +597,7 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "ReviewRequestRemovedEvent": - event.Kind = prx.EventKindReviewRequestRemoved + event.Kind = types.EventKindReviewRequestRemoved if reviewer, ok := item["requestedReviewer"].(map[string]any); ok { if login, ok := reviewer["login"].(string); ok { event.Target = login @@ -607,53 +607,53 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "MentionedEvent": - event.Kind = prx.EventKindMentioned + event.Kind = types.EventKindMentioned event.Body = "User was mentioned" case "ReadyForReviewEvent": - event.Kind = prx.EventKindReadyForReview + event.Kind = types.EventKindReadyForReview case "ConvertToDraftEvent": - event.Kind = prx.EventKindConvertToDraft + event.Kind = types.EventKindConvertToDraft case "ClosedEvent": - event.Kind = prx.EventKindClosed + event.Kind = types.EventKindClosed case "ReopenedEvent": - event.Kind = prx.EventKindReopened + event.Kind = types.EventKindReopened case "MergedEvent": - event.Kind = prx.EventKindMerged + event.Kind = types.EventKindMerged case "AutoMergeEnabledEvent": - event.Kind = prx.EventKindAutoMergeEnabled + event.Kind = types.EventKindAutoMergeEnabled case "AutoMergeDisabledEvent": - event.Kind = prx.EventKindAutoMergeDisabled + event.Kind = types.EventKindAutoMergeDisabled case "ReviewDismissedEvent": - event.Kind = prx.EventKindReviewDismissed + event.Kind = types.EventKindReviewDismissed if msg, ok := item["dismissalMessage"].(string); ok { event.Body = msg } case "BaseRefChangedEvent": - event.Kind = prx.EventKindBaseRefChanged + event.Kind = types.EventKindBaseRefChanged case "BaseRefForcePushedEvent": - event.Kind = prx.EventKindBaseRefForcePushed + event.Kind = types.EventKindBaseRefForcePushed case "HeadRefForcePushedEvent": - event.Kind = prx.EventKindHeadRefForcePushed + event.Kind = types.EventKindHeadRefForcePushed case "HeadRefDeletedEvent": - event.Kind = prx.EventKindHeadRefDeleted + event.Kind = types.EventKindHeadRefDeleted case "HeadRefRestoredEvent": - event.Kind = prx.EventKindHeadRefRestored + event.Kind = types.EventKindHeadRefRestored case "RenamedTitleEvent": - event.Kind = prx.EventKindRenamedTitle + event.Kind = types.EventKindRenamedTitle if prev, ok := item["previousTitle"].(string); ok { if curr, ok := item["currentTitle"].(string); ok { event.Body = fmt.Sprintf("Renamed from %q to %q", prev, curr) @@ -661,58 +661,58 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an } case "LockedEvent": - event.Kind = prx.EventKindLocked + event.Kind = types.EventKindLocked case "UnlockedEvent": - event.Kind = prx.EventKindUnlocked + event.Kind = types.EventKindUnlocked case "AddedToMergeQueueEvent": - event.Kind = prx.EventKindAddedToMergeQueue + event.Kind = types.EventKindAddedToMergeQueue case "RemovedFromMergeQueueEvent": - event.Kind = prx.EventKindRemovedFromMergeQueue + event.Kind = types.EventKindRemovedFromMergeQueue case "AutomaticBaseChangeSucceededEvent": - event.Kind = prx.EventKindAutomaticBaseChangeSucceeded + event.Kind = types.EventKindAutomaticBaseChangeSucceeded case "AutomaticBaseChangeFailedEvent": - event.Kind = prx.EventKindAutomaticBaseChangeFailed + event.Kind = types.EventKindAutomaticBaseChangeFailed case "ConnectedEvent": - event.Kind = prx.EventKindConnected + event.Kind = types.EventKindConnected case "DisconnectedEvent": - event.Kind = prx.EventKindDisconnected + event.Kind = types.EventKindDisconnected case "CrossReferencedEvent": - event.Kind = prx.EventKindCrossReferenced + event.Kind = types.EventKindCrossReferenced case "ReferencedEvent": - event.Kind = prx.EventKindReferenced + event.Kind = types.EventKindReferenced case "SubscribedEvent": - event.Kind = prx.EventKindSubscribed + event.Kind = types.EventKindSubscribed case "UnsubscribedEvent": - event.Kind = prx.EventKindUnsubscribed + event.Kind = types.EventKindUnsubscribed case "DeployedEvent": - event.Kind = prx.EventKindDeployed + event.Kind = types.EventKindDeployed case "DeploymentEnvironmentChangedEvent": - event.Kind = prx.EventKindDeploymentEnvironmentChanged + event.Kind = types.EventKindDeploymentEnvironmentChanged case "PinnedEvent": - event.Kind = prx.EventKindPinned + event.Kind = types.EventKindPinned case "UnpinnedEvent": - event.Kind = prx.EventKindUnpinned + event.Kind = types.EventKindUnpinned case "TransferredEvent": - event.Kind = prx.EventKindTransferred + event.Kind = types.EventKindTransferred case "UserBlockedEvent": - event.Kind = prx.EventKindUserBlocked + event.Kind = types.EventKindUserBlocked default: return nil @@ -724,18 +724,18 @@ func (*Platform) parseGraphQLTimelineEvent(_ context.Context, item map[string]an // writeAccessFromAssociation calculates write access from association. func (p *Platform) writeAccessFromAssociation(ctx context.Context, owner, repo, user, association string) int { if user == "" { - return prx.WriteAccessNA + return types.WriteAccessNA } switch association { case "OWNER", "COLLABORATOR": - return prx.WriteAccessDefinitely + return types.WriteAccessDefinitely case "MEMBER": return p.checkCollaboratorPermission(ctx, owner, repo, user) case "CONTRIBUTOR", "NONE", "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER": - return prx.WriteAccessUnlikely + return types.WriteAccessUnlikely default: - return prx.WriteAccessNA + return types.WriteAccessNA } } @@ -754,16 +754,16 @@ func (p *Platform) checkCollaboratorPermission(ctx context.Context, owner, repo, return result, nil }) if err != nil { - return prx.WriteAccessLikely + return types.WriteAccessLikely } switch collabs[user] { case "admin", "maintain", "write": - return prx.WriteAccessDefinitely + return types.WriteAccessDefinitely case "read", "triage", "none": - return prx.WriteAccessNo + return types.WriteAccessNo default: - return prx.WriteAccessUnlikely + return types.WriteAccessUnlikely } } @@ -848,15 +848,15 @@ func truncateSHA(sha string) string { } // buildReviewersMap constructs a map of reviewer login to their review state. -func buildReviewersMap(data *graphQLPullRequestComplete) map[string]prx.ReviewState { - reviewers := make(map[string]prx.ReviewState) +func buildReviewersMap(data *graphQLPullRequestComplete) map[string]types.ReviewState { + reviewers := make(map[string]types.ReviewState) for _, request := range data.ReviewRequests.Nodes { reviewer := request.RequestedReviewer if reviewer.Login != "" { - reviewers[reviewer.Login] = prx.ReviewStatePending + reviewers[reviewer.Login] = types.ReviewStatePending } else if reviewer.Name != "" { - reviewers[reviewer.Name] = prx.ReviewStatePending + reviewers[reviewer.Name] = types.ReviewStatePending } } @@ -866,14 +866,14 @@ func buildReviewersMap(data *graphQLPullRequestComplete) map[string]prx.ReviewSt continue } - var state prx.ReviewState + var state types.ReviewState switch strings.ToUpper(review.State) { case "APPROVED": - state = prx.ReviewStateApproved + state = types.ReviewStateApproved case "CHANGES_REQUESTED": - state = prx.ReviewStateChangesRequested + state = types.ReviewStateChangesRequested case "COMMENTED": - state = prx.ReviewStateCommented + state = types.ReviewStateCommented default: continue } @@ -916,7 +916,7 @@ func (p *Platform) fetchRulesetsREST(ctx context.Context, owner, repo string) ([ } // fetchCheckRunsREST fetches check runs via REST API for a specific commit. -func (p *Platform) fetchCheckRunsREST(ctx context.Context, owner, repo, sha string, refTime time.Time) ([]prx.Event, error) { +func (p *Platform) fetchCheckRunsREST(ctx context.Context, owner, repo, sha string, refTime time.Time) ([]types.Event, error) { if sha == "" { return nil, nil } @@ -940,7 +940,7 @@ func (p *Platform) fetchCheckRunsREST(ctx context.Context, owner, repo, sha stri return nil, fmt.Errorf("fetching check runs: %w", err) } - var events []prx.Event + var events []types.Event for _, run := range checkRuns.CheckRuns { if run == nil { continue @@ -960,8 +960,8 @@ func (p *Platform) fetchCheckRunsREST(ctx context.Context, owner, repo, sha stri continue } - event := prx.Event{ - Kind: prx.EventKindCheckRun, + event := types.Event{ + Kind: types.EventKindCheckRun, Timestamp: timestamp, Actor: "github", Bot: true, @@ -995,7 +995,7 @@ func (p *Platform) fetchCheckRunsREST(ctx context.Context, owner, repo, sha stri } // fetchAllCheckRunsREST fetches check runs for all commits in the PR. -func (p *Platform) fetchAllCheckRunsREST(ctx context.Context, owner, repo string, prData *prx.PullRequestData, refTime time.Time) []prx.Event { +func (p *Platform) fetchAllCheckRunsREST(ctx context.Context, owner, repo string, prData *types.PullRequestData, refTime time.Time) []types.Event { shas := make(map[string]bool) if prData.PullRequest.HeadSHA != "" { @@ -1004,12 +1004,12 @@ func (p *Platform) fetchAllCheckRunsREST(ctx context.Context, owner, repo string for i := range prData.Events { e := &prData.Events[i] - if e.Kind == prx.EventKindCommit && e.Body != "" { + if e.Kind == types.EventKindCommit && e.Body != "" { shas[e.Body] = true } } - var all []prx.Event + var all []types.Event seen := make(map[string]bool) for sha := range shas { @@ -1034,12 +1034,12 @@ func (p *Platform) fetchAllCheckRunsREST(ctx context.Context, owner, repo string } // existingRequiredChecks extracts required checks that were already identified. -func (*Platform) existingRequiredChecks(prData *prx.PullRequestData) []string { +func (*Platform) existingRequiredChecks(prData *types.PullRequestData) []string { var required []string for i := range prData.Events { e := &prData.Events[i] - if e.Required && (e.Kind == prx.EventKindCheckRun || e.Kind == prx.EventKindStatusCheck) { + if e.Required && (e.Kind == types.EventKindCheckRun || e.Kind == types.EventKindStatusCheck) { required = append(required, e.Body) } } @@ -1056,7 +1056,7 @@ func (*Platform) existingRequiredChecks(prData *prx.PullRequestData) []string { } // recalculateCheckSummaryWithCheckRuns updates the check summary with REST-fetched check runs. -func (p *Platform) recalculateCheckSummaryWithCheckRuns(_ context.Context, prData *prx.PullRequestData, _ []prx.Event) { +func (p *Platform) recalculateCheckSummaryWithCheckRuns(_ context.Context, prData *types.PullRequestData, _ []types.Event) { var required []string if prData.PullRequest.CheckSummary != nil { for chk := range prData.PullRequest.CheckSummary.Pending { @@ -1064,27 +1064,27 @@ func (p *Platform) recalculateCheckSummaryWithCheckRuns(_ context.Context, prDat } } - prData.PullRequest.CheckSummary = prx.CalculateCheckSummary(prData.Events, required) + prData.PullRequest.CheckSummary = types.CalculateCheckSummary(prData.Events, required) prData.PullRequest.TestState = p.calculateTestStateFromCheckSummary(prData.PullRequest.CheckSummary) } // calculateTestStateFromCheckSummary determines test state from a CheckSummary. -func (*Platform) calculateTestStateFromCheckSummary(summary *prx.CheckSummary) string { +func (*Platform) calculateTestStateFromCheckSummary(summary *types.CheckSummary) string { if summary == nil { - return prx.TestStateNone + return types.TestStateNone } if len(summary.Failing) > 0 { - return prx.TestStateFailing + return types.TestStateFailing } if len(summary.Pending) > 0 { - return prx.TestStatePending + return types.TestStatePending } if len(summary.Success) > 0 { - return prx.TestStatePassing + return types.TestStatePassing } - return prx.TestStateNone + return types.TestStateNone } diff --git a/pkg/prx/github/timeline_events_test.go b/pkg/prx/github/timeline_events_test.go index 00d8bbc..9bcba3d 100644 --- a/pkg/prx/github/timeline_events_test.go +++ b/pkg/prx/github/timeline_events_test.go @@ -7,7 +7,7 @@ import ( "testing" "time" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // TestAutoMergeEventIntegration tests that we properly parse auto_merge_enabled @@ -19,7 +19,7 @@ func TestAutoMergeEventIntegration(t *testing.T) { t.Fatalf("Failed to read test data: %v", err) } - var prData prx.PullRequestData + var prData types.PullRequestData if err := json.Unmarshal(data, &prData); err != nil { t.Fatalf("Failed to unmarshal test data: %v", err) } @@ -30,7 +30,7 @@ func TestAutoMergeEventIntegration(t *testing.T) { } // Find the auto_merge_enabled event - var autoMergeEvent *prx.Event + var autoMergeEvent *types.Event for i := range prData.Events { if prData.Events[i].Kind == "auto_merge_enabled" { autoMergeEvent = &prData.Events[i] diff --git a/pkg/prx/gitlab/platform.go b/pkg/prx/gitlab/platform.go index 7d93f04..4de224c 100644 --- a/pkg/prx/gitlab/platform.go +++ b/pkg/prx/gitlab/platform.go @@ -14,7 +14,7 @@ import ( "time" "github.com/codeGROOVE-dev/fido" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // Cache TTL constants. @@ -114,11 +114,11 @@ func NewPlatform(token string, opts ...Option) *Platform { // Name returns the platform identifier. func (*Platform) Name() string { - return prx.PlatformGitLab + return types.PlatformGitLab } // FetchPR retrieves a merge request with all events and metadata. -func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*prx.PullRequestData, error) { +func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*types.PullRequestData, error) { projectPath := fmt.Sprintf("%s/%s", owner, repo) p.logger.Info("fetching merge request via GitLab REST API", "project", projectPath, "mr", number) @@ -170,9 +170,9 @@ func (p *Platform) FetchPR(ctx context.Context, owner, repo string, number int, // Finalize the pull request with calculated summaries. // Pass the TestState we derived from the pipeline so it doesn't get overwritten. - prx.FinalizePullRequest(&pr, events, nil, pr.TestState) + types.FinalizePullRequest(&pr, events, nil, pr.TestState) - return &prx.PullRequestData{ + return &types.PullRequestData{ CachedAt: time.Now(), PullRequest: pr, Events: events, @@ -493,8 +493,8 @@ func (p *Platform) doRequest(ctx context.Context, url string, result any) (err e // Conversion methods. -func convertMergeRequest(mr *mergeRequest, approvals *approvals, commits []commit) prx.PullRequest { - pr := prx.PullRequest{ +func convertMergeRequest(mr *mergeRequest, approvals *approvals, commits []commit) types.PullRequest { + pr := types.PullRequest{ Number: mr.IID, Title: mr.Title, Body: mr.Description, @@ -535,7 +535,7 @@ func convertMergeRequest(mr *mergeRequest, approvals *approvals, commits []commi } // Set reviewers with their states. - pr.Reviewers = make(map[string]prx.ReviewState) + pr.Reviewers = make(map[string]types.ReviewState) for _, r := range mr.Reviewers { pr.Reviewers[r.Username] = convertReviewerState(r.State) } @@ -543,7 +543,7 @@ func convertMergeRequest(mr *mergeRequest, approvals *approvals, commits []commi // Update reviewer states from approvals. if approvals != nil { for _, ab := range approvals.ApprovedBy { - pr.Reviewers[ab.User.Username] = prx.ReviewStateApproved + pr.Reviewers[ab.User.Username] = types.ReviewStateApproved } } @@ -575,18 +575,18 @@ func isBot(u *user) bool { func convertPipelineToTestState(status string) string { switch status { case "success": - return prx.TestStatePassing + return types.TestStatePassing case "failed": - return prx.TestStateFailing + return types.TestStateFailing case "running": - return prx.TestStateRunning + return types.TestStateRunning case "pending", "waiting_for_resource", "preparing": - return prx.TestStatePending + return types.TestStatePending case "created", "scheduled": - return prx.TestStateQueued + return types.TestStateQueued default: // canceled, skipped, manual, or unknown status - return prx.TestStateNone + return types.TestStateNone } } @@ -601,12 +601,12 @@ func convertState(state string) string { } } -func convertReviewerState(state string) prx.ReviewState { +func convertReviewerState(state string) types.ReviewState { switch state { case "reviewed": - return prx.ReviewStateCommented + return types.ReviewStateCommented default: - return prx.ReviewStatePending + return types.ReviewStatePending } } @@ -642,21 +642,21 @@ func convertToEvents( discussions []discussion, pipelines []pipeline, commits []commit, -) []prx.Event { - var events []prx.Event +) []types.Event { + var events []types.Event // Add MR opened event. - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: mr.CreatedAt, - Kind: prx.EventKindPROpened, + Kind: types.EventKindPROpened, Actor: mr.Author.Username, }) // Add commit events. for i := range commits { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: commits[i].AuthoredDate, - Kind: prx.EventKindCommit, + Kind: types.EventKindCommit, Actor: commits[i].AuthorName, Body: commits[i].ShortID, Description: commits[i].Title, @@ -705,142 +705,142 @@ func convertToEvents( // Add closed/merged events. if mr.MergedAt != nil { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *mr.MergedAt, - Kind: prx.EventKindPRMerged, + Kind: types.EventKindPRMerged, Actor: safeUsername(mr.MergedBy), }) } else if mr.ClosedAt != nil && mr.State == "closed" { - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *mr.ClosedAt, - Kind: prx.EventKindPRClosed, + Kind: types.EventKindPRClosed, }) } return events } -func convertNote(n *note) *prx.Event { +func convertNote(n *note) *types.Event { if n.System { return convertSystemNote(n) } // Regular user comment. - return &prx.Event{ + return &types.Event{ Timestamp: n.CreatedAt, - Kind: prx.EventKindComment, + Kind: types.EventKindComment, Actor: n.Author.Username, Body: n.Body, - Question: prx.ContainsQuestion(n.Body), + Question: types.ContainsQuestion(n.Body), Outdated: n.Resolved, } } -func convertSystemNote(systemNote *note) *prx.Event { +func convertSystemNote(systemNote *note) *types.Event { body := strings.ToLower(systemNote.Body) // Map GitLab system notes to our event kinds. switch { case strings.HasPrefix(body, "approved this merge request"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindReview, + Kind: types.EventKindReview, Actor: systemNote.Author.Username, Outcome: "approved", } case strings.HasPrefix(body, "unapproved this merge request"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindReview, + Kind: types.EventKindReview, Actor: systemNote.Author.Username, Outcome: "dismissed", } case strings.HasPrefix(body, "requested review from"): target := extractMentionFromNote(systemNote.Body) - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindReviewRequested, + Kind: types.EventKindReviewRequested, Actor: systemNote.Author.Username, Target: target, } case strings.HasPrefix(body, "assigned to"): target := extractMentionFromNote(systemNote.Body) - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindAssigned, + Kind: types.EventKindAssigned, Actor: systemNote.Author.Username, Target: target, } case strings.HasPrefix(body, "unassigned"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindUnassigned, + Kind: types.EventKindUnassigned, Actor: systemNote.Author.Username, } case strings.HasPrefix(body, "added") && strings.Contains(body, "label"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindLabeled, + Kind: types.EventKindLabeled, Actor: systemNote.Author.Username, Description: systemNote.Body, } case strings.HasPrefix(body, "removed") && strings.Contains(body, "label"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindUnlabeled, + Kind: types.EventKindUnlabeled, Actor: systemNote.Author.Username, Description: systemNote.Body, } case strings.HasPrefix(body, "marked as a draft"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindConvertToDraft, + Kind: types.EventKindConvertToDraft, Actor: systemNote.Author.Username, } case strings.HasPrefix(body, "marked this merge request as ready"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindReadyForReview, + Kind: types.EventKindReadyForReview, Actor: systemNote.Author.Username, } case strings.HasPrefix(body, "changed target branch"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindBaseRefChanged, + Kind: types.EventKindBaseRefChanged, Actor: systemNote.Author.Username, Description: systemNote.Body, } case strings.HasPrefix(body, "mentioned in"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindCrossReferenced, + Kind: types.EventKindCrossReferenced, Actor: systemNote.Author.Username, Description: systemNote.Body, } case strings.HasPrefix(body, "closed"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindClosed, + Kind: types.EventKindClosed, Actor: systemNote.Author.Username, } case strings.HasPrefix(body, "reopened"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindReopened, + Kind: types.EventKindReopened, Actor: systemNote.Author.Username, } case strings.HasPrefix(body, "changed title"): - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindRenamedTitle, + Kind: types.EventKindRenamedTitle, Actor: systemNote.Author.Username, Description: systemNote.Body, } default: // Unknown system note - include as a generic comment. - return &prx.Event{ + return &types.Event{ Timestamp: systemNote.CreatedAt, - Kind: prx.EventKindComment, + Kind: types.EventKindComment, Actor: systemNote.Author.Username, Body: systemNote.Body, Description: "system", @@ -848,8 +848,8 @@ func convertSystemNote(systemNote *note) *prx.Event { } } -func convertPipeline(p *pipeline) []prx.Event { - var events []prx.Event +func convertPipeline(p *pipeline) []types.Event { + var events []types.Event // Add pipeline started event. if p.StartedAt != nil { @@ -857,9 +857,9 @@ func convertPipeline(p *pipeline) []prx.Event { if p.Status == "running" { status = "running" } - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *p.StartedAt, - Kind: prx.EventKindCheckRun, + Kind: types.EventKindCheckRun, Body: fmt.Sprintf("pipeline-%d", p.ID), Outcome: status, Bot: true, @@ -870,9 +870,9 @@ func convertPipeline(p *pipeline) []prx.Event { // Add pipeline completed event. if p.FinishedAt != nil { outcome := convertPipelineStatus(p.Status) - events = append(events, prx.Event{ + events = append(events, types.Event{ Timestamp: *p.FinishedAt, - Kind: prx.EventKindCheckRun, + Kind: types.EventKindCheckRun, Body: fmt.Sprintf("pipeline-%d", p.ID), Outcome: outcome, Bot: true, diff --git a/pkg/prx/gitlab/platform_test.go b/pkg/prx/gitlab/platform_test.go index d9793fe..fa59b5d 100644 --- a/pkg/prx/gitlab/platform_test.go +++ b/pkg/prx/gitlab/platform_test.go @@ -9,13 +9,13 @@ import ( "testing" "time" - "github.com/codeGROOVE-dev/prx/pkg/prx" + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestPlatform_Name(t *testing.T) { p := NewPlatform("token") - if got := p.Name(); got != prx.PlatformGitLab { - t.Errorf("Name() = %q, want %q", got, prx.PlatformGitLab) + if got := p.Name(); got != types.PlatformGitLab { + t.Errorf("Name() = %q, want %q", got, types.PlatformGitLab) } } @@ -225,8 +225,8 @@ func TestPlatform_FetchPR(t *testing.T) { if pr.HeadSHA != "abc123def456" { t.Errorf("HeadSHA = %q, want %q", pr.HeadSHA, "abc123def456") } - if pr.TestState != prx.TestStatePassing { - t.Errorf("TestState = %q, want %q", pr.TestState, prx.TestStatePassing) + if pr.TestState != types.TestStatePassing { + t.Errorf("TestState = %q, want %q", pr.TestState, types.TestStatePassing) } // Verify labels @@ -240,8 +240,8 @@ func TestPlatform_FetchPR(t *testing.T) { } // Verify reviewers - if pr.Reviewers["approver1"] != prx.ReviewStateApproved { - t.Errorf("Reviewers[approver1] = %v, want %v", pr.Reviewers["approver1"], prx.ReviewStateApproved) + if pr.Reviewers["approver1"] != types.ReviewStateApproved { + t.Errorf("Reviewers[approver1] = %v, want %v", pr.Reviewers["approver1"], types.ReviewStateApproved) } // Verify commits @@ -259,7 +259,7 @@ func TestPlatform_FetchPR(t *testing.T) { for _, e := range data.Events { eventTypes[e.Kind] = true } - expectedTypes := []string{prx.EventKindPROpened, prx.EventKindCommit, prx.EventKindComment} + expectedTypes := []string{types.EventKindPROpened, types.EventKindCommit, types.EventKindComment} for _, et := range expectedTypes { if !eventTypes[et] { t.Errorf("Missing event type %q in events", et) @@ -342,7 +342,7 @@ func TestPlatform_FetchPR_Merged(t *testing.T) { // Check for merged event hasMergedEvent := false for _, e := range data.Events { - if e.Kind == prx.EventKindPRMerged { + if e.Kind == types.EventKindPRMerged { hasMergedEvent = true if e.Actor != "merger" { t.Errorf("Merged event actor = %q, want %q", e.Actor, "merger") @@ -476,8 +476,8 @@ func TestPlatform_FetchPR_FailingPipeline(t *testing.T) { t.Fatalf("FetchPR() error = %v", err) } - if data.PullRequest.TestState != prx.TestStateFailing { - t.Errorf("TestState = %q, want %q", data.PullRequest.TestState, prx.TestStateFailing) + if data.PullRequest.TestState != types.TestStateFailing { + t.Errorf("TestState = %q, want %q", data.PullRequest.TestState, types.TestStateFailing) } } @@ -560,11 +560,11 @@ func TestConvertState(t *testing.T) { func TestConvertReviewerState(t *testing.T) { tests := []struct { input string - want prx.ReviewState + want types.ReviewState }{ - {"reviewed", prx.ReviewStateCommented}, - {"unreviewed", prx.ReviewStatePending}, - {"", prx.ReviewStatePending}, + {"reviewed", types.ReviewStateCommented}, + {"unreviewed", types.ReviewStatePending}, + {"", types.ReviewStatePending}, } for _, tt := range tests { @@ -582,18 +582,18 @@ func TestConvertPipelineToTestState(t *testing.T) { input string want string }{ - {"success", prx.TestStatePassing}, - {"failed", prx.TestStateFailing}, - {"running", prx.TestStateRunning}, - {"pending", prx.TestStatePending}, - {"waiting_for_resource", prx.TestStatePending}, - {"preparing", prx.TestStatePending}, - {"created", prx.TestStateQueued}, - {"scheduled", prx.TestStateQueued}, - {"canceled", prx.TestStateNone}, - {"skipped", prx.TestStateNone}, - {"manual", prx.TestStateNone}, - {"unknown", prx.TestStateNone}, + {"success", types.TestStatePassing}, + {"failed", types.TestStateFailing}, + {"running", types.TestStateRunning}, + {"pending", types.TestStatePending}, + {"waiting_for_resource", types.TestStatePending}, + {"preparing", types.TestStatePending}, + {"created", types.TestStateQueued}, + {"scheduled", types.TestStateQueued}, + {"canceled", types.TestStateNone}, + {"skipped", types.TestStateNone}, + {"manual", types.TestStateNone}, + {"unknown", types.TestStateNone}, } for _, tt := range tests { @@ -749,77 +749,77 @@ func TestConvertSystemNote(t *testing.T) { { name: "approved", n: ¬e{Body: "approved this merge request", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindReview, + wantKind: types.EventKindReview, }, { name: "unapproved", n: ¬e{Body: "unapproved this merge request", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindReview, + wantKind: types.EventKindReview, }, { name: "requested review", n: ¬e{Body: "requested review from @reviewer", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindReviewRequested, + wantKind: types.EventKindReviewRequested, }, { name: "assigned", n: ¬e{Body: "assigned to @assignee", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindAssigned, + wantKind: types.EventKindAssigned, }, { name: "unassigned", n: ¬e{Body: "unassigned @user", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindUnassigned, + wantKind: types.EventKindUnassigned, }, { name: "added label", n: ¬e{Body: "added ~bug label", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindLabeled, + wantKind: types.EventKindLabeled, }, { name: "removed label", n: ¬e{Body: "removed ~bug label", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindUnlabeled, + wantKind: types.EventKindUnlabeled, }, { name: "marked as draft", n: ¬e{Body: "marked as a draft", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindConvertToDraft, + wantKind: types.EventKindConvertToDraft, }, { name: "marked ready", n: ¬e{Body: "marked this merge request as ready", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindReadyForReview, + wantKind: types.EventKindReadyForReview, }, { name: "changed target branch", n: ¬e{Body: "changed target branch from main to develop", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindBaseRefChanged, + wantKind: types.EventKindBaseRefChanged, }, { name: "mentioned in", n: ¬e{Body: "mentioned in issue #123", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindCrossReferenced, + wantKind: types.EventKindCrossReferenced, }, { name: "closed", n: ¬e{Body: "closed", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindClosed, + wantKind: types.EventKindClosed, }, { name: "reopened", n: ¬e{Body: "reopened", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindReopened, + wantKind: types.EventKindReopened, }, { name: "changed title", n: ¬e{Body: "changed title from old to new", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindRenamedTitle, + wantKind: types.EventKindRenamedTitle, }, { name: "unknown system note", n: ¬e{Body: "some unknown system action", Author: author, CreatedAt: now, System: true}, - wantKind: prx.EventKindComment, + wantKind: types.EventKindComment, }, } @@ -854,8 +854,8 @@ func TestConvertNote(t *testing.T) { System: false, } got := convertNote(n) - if got.Kind != prx.EventKindComment { - t.Errorf("Kind = %q, want %q", got.Kind, prx.EventKindComment) + if got.Kind != types.EventKindComment { + t.Errorf("Kind = %q, want %q", got.Kind, types.EventKindComment) } if got.Actor != "commenter" { t.Errorf("Actor = %q, want %q", got.Actor, "commenter") @@ -897,8 +897,8 @@ func TestConvertNote(t *testing.T) { System: true, } got := convertNote(n) - if got.Kind != prx.EventKindReview { - t.Errorf("Kind = %q, want %q", got.Kind, prx.EventKindReview) + if got.Kind != types.EventKindReview { + t.Errorf("Kind = %q, want %q", got.Kind, types.EventKindReview) } }) } @@ -1083,11 +1083,11 @@ func TestConvertMergeRequest(t *testing.T) { if len(result.Commits) != 2 { t.Errorf("len(Commits) = %d, want 2", len(result.Commits)) } - if result.Reviewers["approver1"] != prx.ReviewStateApproved { + if result.Reviewers["approver1"] != types.ReviewStateApproved { t.Errorf("Reviewers[approver1] = %v, want approved", result.Reviewers["approver1"]) } - if result.TestState != prx.TestStatePassing { - t.Errorf("TestState = %q, want %q", result.TestState, prx.TestStatePassing) + if result.TestState != types.TestStatePassing { + t.Errorf("TestState = %q, want %q", result.TestState, types.TestStatePassing) } } diff --git a/pkg/prx/participant_access_test.go b/pkg/prx/participant_access_test.go index b991220..8ab3e84 100644 --- a/pkg/prx/participant_access_test.go +++ b/pkg/prx/participant_access_test.go @@ -3,75 +3,77 @@ package prx import ( "testing" "time" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // TestCalculateParticipantAccess tests participant access map building func TestCalculateParticipantAccess(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ Author: "author1", - AuthorWriteAccess: WriteAccessUnlikely, + AuthorWriteAccess: types.WriteAccessUnlikely, Assignees: []string{"assignee1", "assignee2"}, - Reviewers: map[string]ReviewState{ - "reviewer1": ReviewStateApproved, - "reviewer2": ReviewStatePending, + Reviewers: map[string]types.ReviewState{ + "reviewer1": types.ReviewStateApproved, + "reviewer2": types.ReviewStatePending, }, } - events := []Event{ + events := []types.Event{ { Kind: "pr_opened", Actor: "author1", - WriteAccess: WriteAccessUnlikely, + WriteAccess: types.WriteAccessUnlikely, Timestamp: time.Now(), }, { Kind: "review", Actor: "reviewer1", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, Timestamp: time.Now(), }, { Kind: "comment", Actor: "commenter1", - WriteAccess: WriteAccessLikely, + WriteAccess: types.WriteAccessLikely, Timestamp: time.Now(), }, { Kind: "labeled", Actor: "labeler1", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, Timestamp: time.Now(), }, } - participants := CalculateParticipantAccess(events, pr) + participants := types.CalculateParticipantAccess(events, pr) // Verify author is included - if access, ok := participants["author1"]; !ok || access != WriteAccessUnlikely { + if access, ok := participants["author1"]; !ok || access != types.WriteAccessUnlikely { t.Errorf("Expected author1 with WriteAccessUnlikely, got %v", access) } // Verify assignees are included with WriteAccessNA - if access, ok := participants["assignee1"]; !ok || access != WriteAccessNA { + if access, ok := participants["assignee1"]; !ok || access != types.WriteAccessNA { t.Errorf("Expected assignee1 with WriteAccessNA, got %v", access) } - if access, ok := participants["assignee2"]; !ok || access != WriteAccessNA { + if access, ok := participants["assignee2"]; !ok || access != types.WriteAccessNA { t.Errorf("Expected assignee2 with WriteAccessNA, got %v", access) } // Verify reviewers are included (reviewer1 gets upgraded from NA to Definitely by event) - if access, ok := participants["reviewer1"]; !ok || access != WriteAccessDefinitely { + if access, ok := participants["reviewer1"]; !ok || access != types.WriteAccessDefinitely { t.Errorf("Expected reviewer1 with WriteAccessDefinitely, got %v", access) } - if access, ok := participants["reviewer2"]; !ok || access != WriteAccessNA { + if access, ok := participants["reviewer2"]; !ok || access != types.WriteAccessNA { t.Errorf("Expected reviewer2 with WriteAccessNA, got %v", access) } // Verify event actors are included - if access, ok := participants["commenter1"]; !ok || access != WriteAccessLikely { + if access, ok := participants["commenter1"]; !ok || access != types.WriteAccessLikely { t.Errorf("Expected commenter1 with WriteAccessLikely, got %v", access) } - if access, ok := participants["labeler1"]; !ok || access != WriteAccessDefinitely { + if access, ok := participants["labeler1"]; !ok || access != types.WriteAccessDefinitely { t.Errorf("Expected labeler1 with WriteAccessDefinitely, got %v", access) } @@ -84,51 +86,51 @@ func TestCalculateParticipantAccess(t *testing.T) { // TestCalculateParticipantAccessUpgrade tests that write access gets upgraded func TestCalculateParticipantAccessUpgrade(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ Author: "user1", - AuthorWriteAccess: WriteAccessLikely, + AuthorWriteAccess: types.WriteAccessLikely, } - events := []Event{ + events := []types.Event{ { Kind: "pr_opened", Actor: "user1", - WriteAccess: WriteAccessLikely, + WriteAccess: types.WriteAccessLikely, Timestamp: time.Now(), }, { Kind: "labeled", Actor: "user1", - WriteAccess: WriteAccessDefinitely, // Upgraded after label action + WriteAccess: types.WriteAccessDefinitely, // Upgraded after label action Timestamp: time.Now().Add(time.Minute), }, } - participants := CalculateParticipantAccess(events, pr) + participants := types.CalculateParticipantAccess(events, pr) // Verify user1 got upgraded to WriteAccessDefinitely - if access, ok := participants["user1"]; !ok || access != WriteAccessDefinitely { + if access, ok := participants["user1"]; !ok || access != types.WriteAccessDefinitely { t.Errorf("Expected user1 to be upgraded to WriteAccessDefinitely, got %v", access) } } // TestCalculateParticipantAccessEmpty tests empty PR func TestCalculateParticipantAccessEmpty(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ Author: "author1", - AuthorWriteAccess: WriteAccessUnlikely, + AuthorWriteAccess: types.WriteAccessUnlikely, } - events := []Event{} + events := []types.Event{} - participants := CalculateParticipantAccess(events, pr) + participants := types.CalculateParticipantAccess(events, pr) // Should only have the author if len(participants) != 1 { t.Errorf("Expected 1 participant, got %d", len(participants)) } - if access, ok := participants["author1"]; !ok || access != WriteAccessUnlikely { + if access, ok := participants["author1"]; !ok || access != types.WriteAccessUnlikely { t.Errorf("Expected author1 with WriteAccessUnlikely, got %v", access) } } diff --git a/pkg/prx/platform.go b/pkg/prx/platform.go deleted file mode 100644 index ccb25e1..0000000 --- a/pkg/prx/platform.go +++ /dev/null @@ -1,17 +0,0 @@ -package prx - -import ( - "context" - "time" -) - -// Platform fetches pull request data from a code hosting service. -// Each platform (GitHub, GitLab, Codeberg) implements its own fetching strategy. -type Platform interface { - // FetchPR retrieves a pull request with all events and metadata. - // The refTime parameter is used for cache validation decisions. - FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*PullRequestData, error) - - // Name returns the platform identifier (e.g., "github", "gitlab", "codeberg"). - Name() string -} diff --git a/pkg/prx/pullrequest_extended_test.go b/pkg/prx/pullrequest_extended_test.go index 1ebeda8..69db514 100644 --- a/pkg/prx/pullrequest_extended_test.go +++ b/pkg/prx/pullrequest_extended_test.go @@ -3,6 +3,8 @@ package prx import ( "testing" "time" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestFinalizePullRequest(t *testing.T) { @@ -10,8 +12,8 @@ func TestFinalizePullRequest(t *testing.T) { tests := []struct { name string - pr PullRequest - events []Event + pr types.PullRequest + events []types.Event requiredChecks []string testStateFromAPI string wantTestState string @@ -20,30 +22,30 @@ func TestFinalizePullRequest(t *testing.T) { }{ { name: "blocked pr without approvals", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "blocked", }, - events: []Event{}, + events: []types.Event{}, requiredChecks: []string{}, testStateFromAPI: "", - wantTestState: TestStateNone, + wantTestState: types.TestStateNone, wantMergeable: boolPtr(false), wantDescContains: "requires approval", }, { name: "clean pr ready to merge", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "clean", }, - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "reviewer", Timestamp: now, Outcome: "APPROVED", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, { Kind: "status_check", @@ -54,16 +56,16 @@ func TestFinalizePullRequest(t *testing.T) { }, requiredChecks: []string{"test"}, testStateFromAPI: "passing", - wantTestState: TestStatePassing, + wantTestState: types.TestStatePassing, wantDescContains: "ready to merge", }, { name: "unstable pr with failing checks", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "unstable", }, - events: []Event{ + events: []types.Event{ { Kind: "status_check", Timestamp: now, @@ -73,52 +75,52 @@ func TestFinalizePullRequest(t *testing.T) { }, requiredChecks: []string{"test"}, testStateFromAPI: "failing", - wantTestState: TestStateFailing, + wantTestState: types.TestStateFailing, wantDescContains: "status checks are failing", }, { name: "dirty pr with merge conflicts", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "dirty", }, - events: []Event{}, + events: []types.Event{}, requiredChecks: []string{}, testStateFromAPI: "", - wantTestState: TestStateNone, + wantTestState: types.TestStateNone, wantMergeable: boolPtr(false), wantDescContains: "merge conflicts", }, { name: "draft pr", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "draft", Draft: true, }, - events: []Event{}, + events: []types.Event{}, requiredChecks: []string{}, testStateFromAPI: "", - wantTestState: TestStateNone, + wantTestState: types.TestStateNone, wantDescContains: "draft state", }, { name: "unknown mergeable state", - pr: PullRequest{ + pr: types.PullRequest{ Number: 1, MergeableState: "unknown", }, - events: []Event{}, + events: []types.Event{}, requiredChecks: []string{}, testStateFromAPI: "", - wantTestState: TestStateNone, + wantTestState: types.TestStateNone, wantDescContains: "being calculated", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - FinalizePullRequest(&tt.pr, tt.events, tt.requiredChecks, tt.testStateFromAPI) + types.FinalizePullRequest(&tt.pr, tt.events, tt.requiredChecks, tt.testStateFromAPI) if tt.pr.TestState != tt.wantTestState { t.Errorf("TestState = %v, want %v", tt.pr.TestState, tt.wantTestState) @@ -147,54 +149,54 @@ func TestFinalizePullRequest(t *testing.T) { func TestFixTestState(t *testing.T) { tests := []struct { name string - checkSummary *CheckSummary + checkSummary *types.CheckSummary wantTestState string }{ { name: "failing checks", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{"test1": "failed"}, Success: map[string]string{"test2": "passed"}, }, - wantTestState: TestStateFailing, + wantTestState: types.TestStateFailing, }, { name: "cancelled checks", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Cancelled: map[string]string{"test1": "cancelled"}, }, - wantTestState: TestStateFailing, + wantTestState: types.TestStateFailing, }, { name: "pending checks", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Pending: map[string]string{"test1": "pending"}, Success: map[string]string{"test2": "passed"}, }, - wantTestState: TestStatePending, + wantTestState: types.TestStatePending, }, { name: "only success checks", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Success: map[string]string{"test1": "passed", "test2": "passed"}, }, - wantTestState: TestStatePassing, + wantTestState: types.TestStatePassing, }, { name: "no checks", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Success: map[string]string{}, }, - wantTestState: TestStateNone, + wantTestState: types.TestStateNone, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ CheckSummary: tt.checkSummary, } - FixTestState(pr) + types.FixTestState(pr) if pr.TestState != tt.wantTestState { t.Errorf("TestState = %v, want %v", pr.TestState, tt.wantTestState) } @@ -206,18 +208,18 @@ func TestSetMergeableDescription(t *testing.T) { tests := []struct { name string mergeableState string - checkSummary *CheckSummary - approvalSummary *ApprovalSummary + checkSummary *types.CheckSummary + approvalSummary *types.ApprovalSummary wantDescContains string }{ { name: "blocked state without approvals", mergeableState: "blocked", - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{}, Pending: map[string]string{}, }, - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 0, }, wantDescContains: "requires approval", @@ -256,18 +258,18 @@ func TestSetMergeableDescription(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ MergeableState: tt.mergeableState, CheckSummary: tt.checkSummary, ApprovalSummary: tt.approvalSummary, } if pr.CheckSummary == nil { - pr.CheckSummary = &CheckSummary{} + pr.CheckSummary = &types.CheckSummary{} } if pr.ApprovalSummary == nil { - pr.ApprovalSummary = &ApprovalSummary{} + pr.ApprovalSummary = &types.ApprovalSummary{} } - setMergeableDescription(pr) + types.SetMergeableDescription(pr) if tt.wantDescContains == "" { if pr.MergeableStateDescription != "" { t.Errorf("Expected empty description for unknown state, got %q", pr.MergeableStateDescription) @@ -283,16 +285,16 @@ func TestSetMergeableDescription(t *testing.T) { func TestSetBlockedDescription(t *testing.T) { tests := []struct { name string - approvalSummary *ApprovalSummary - checkSummary *CheckSummary + approvalSummary *types.ApprovalSummary + checkSummary *types.CheckSummary wantDescContains string }{ { name: "no approvals, no checks", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 0, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{}, Pending: map[string]string{}, }, @@ -300,10 +302,10 @@ func TestSetBlockedDescription(t *testing.T) { }, { name: "no approvals with pending checks", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 0, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{}, Pending: map[string]string{"test": "pending"}, }, @@ -311,30 +313,30 @@ func TestSetBlockedDescription(t *testing.T) { }, { name: "failing checks without approval", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 0, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{"test": "failed"}, }, wantDescContains: "failing status checks and requires approval", }, { name: "failing checks with approval", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 1, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{"test": "failed"}, }, wantDescContains: "blocked by failing status checks", }, { name: "pending checks only", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 1, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{}, Pending: map[string]string{"test": "pending"}, }, @@ -342,10 +344,10 @@ func TestSetBlockedDescription(t *testing.T) { }, { name: "has approvals but still blocked", - approvalSummary: &ApprovalSummary{ + approvalSummary: &types.ApprovalSummary{ ApprovalsWithWriteAccess: 1, }, - checkSummary: &CheckSummary{ + checkSummary: &types.CheckSummary{ Failing: map[string]string{}, Pending: map[string]string{}, }, @@ -355,12 +357,12 @@ func TestSetBlockedDescription(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pr := &PullRequest{ + pr := &types.PullRequest{ MergeableState: "blocked", ApprovalSummary: tt.approvalSummary, CheckSummary: tt.checkSummary, } - setBlockedDescription(pr) + types.SetBlockedDescription(pr) if !contains(pr.MergeableStateDescription, tt.wantDescContains) { t.Errorf("MergeableStateDescription = %q, want to contain %q", pr.MergeableStateDescription, tt.wantDescContains) diff --git a/pkg/prx/question_integration_test.go b/pkg/prx/question_integration_test.go index 67fed75..4b8c911 100644 --- a/pkg/prx/question_integration_test.go +++ b/pkg/prx/question_integration_test.go @@ -2,6 +2,8 @@ package prx import ( "testing" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestQuestionFieldIntegration(t *testing.T) { @@ -40,11 +42,11 @@ func TestQuestionFieldIntegration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Simulate creating an event like in reviews - event := Event{ + event := types.Event{ Kind: "review", Body: tt.body, Outcome: tt.outcome, - Question: ContainsQuestion(tt.body), + Question: types.ContainsQuestion(tt.body), } if event.Question != tt.expected { diff --git a/pkg/prx/question_test.go b/pkg/prx/question_test.go index 5036667..51a490f 100644 --- a/pkg/prx/question_test.go +++ b/pkg/prx/question_test.go @@ -2,6 +2,8 @@ package prx import ( "testing" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) //nolint:maintidx // Comprehensive test coverage requires many test cases @@ -367,7 +369,7 @@ func TestContainsQuestion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := ContainsQuestion(tt.input) + result := types.ContainsQuestion(tt.input) if result != tt.expected { t.Errorf("ContainsQuestion(%q) = %v, want %v", tt.input, result, tt.expected) } @@ -387,7 +389,7 @@ func BenchmarkContainsQuestion(b *testing.B) { for b.Loop() { for _, tc := range testCases { - _ = ContainsQuestion(tc) + _ = types.ContainsQuestion(tc) } } } diff --git a/pkg/prx/status_description_test.go b/pkg/prx/status_description_test.go index d8a455b..bf392bc 100644 --- a/pkg/prx/status_description_test.go +++ b/pkg/prx/status_description_test.go @@ -3,6 +3,8 @@ package prx import ( "testing" "time" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) // testCheckRun is a test-only type that mirrors the structure of GitHub check runs. @@ -114,7 +116,7 @@ func TestCheckRunStatusDescriptions(t *testing.T) { timestamp = tt.checkRun.StartedAt } - event := Event{ + event := types.Event{ Kind: "check_run", Timestamp: timestamp, Actor: "github", @@ -147,7 +149,7 @@ func TestCheckRunStatusDescriptions(t *testing.T) { } func TestCalculateCheckSummaryWithDescriptions(t *testing.T) { - events := []Event{ + events := []types.Event{ { Kind: "check_run", Body: "*control", @@ -181,7 +183,7 @@ func TestCalculateCheckSummaryWithDescriptions(t *testing.T) { "*control", } - summary := CalculateCheckSummary(events, requiredChecks) + summary := types.CalculateCheckSummary(events, requiredChecks) // Verify counts if len(summary.Success) != 2 { @@ -228,7 +230,7 @@ func TestDropshotPR1359Regression(t *testing.T) { } // Process into event - event := Event{ + event := types.Event{ Kind: "check_run", Timestamp: checkRun.CompletedAt, Actor: "github", @@ -255,8 +257,8 @@ func TestDropshotPR1359Regression(t *testing.T) { } // Also test that it appears correctly in the check summary - events := []Event{event} - summary := CalculateCheckSummary(events, []string{}) + events := []types.Event{event} + summary := types.CalculateCheckSummary(events, []string{}) if desc, exists := summary.Failing["*control"]; !exists { t.Error("Regression detected: *control not in failing statuses") diff --git a/pkg/prx/event_processing.go b/pkg/prx/types/processing.go similarity index 99% rename from pkg/prx/event_processing.go rename to pkg/prx/types/processing.go index 29514fa..efbc071 100644 --- a/pkg/prx/event_processing.go +++ b/pkg/prx/types/processing.go @@ -1,4 +1,4 @@ -package prx +package types import ( "time" diff --git a/pkg/prx/types/processing_test.go b/pkg/prx/types/processing_test.go new file mode 100644 index 0000000..c6c1989 --- /dev/null +++ b/pkg/prx/types/processing_test.go @@ -0,0 +1,598 @@ +package types + +import ( + "testing" + "time" +) + +func TestFilterEvents(t *testing.T) { + tests := []struct { + name string + events []Event + expected int // expected number of events after filtering + }{ + { + name: "keeps non-status_check events", + events: []Event{ + {Kind: EventKindCommit, Actor: "user1"}, + {Kind: EventKindComment, Actor: "user2"}, + {Kind: EventKindReview, Actor: "user3"}, + }, + expected: 3, + }, + { + name: "filters successful status checks", + events: []Event{ + {Kind: EventKindStatusCheck, Outcome: "success", Body: "check1"}, + {Kind: EventKindComment, Actor: "user1"}, + }, + expected: 1, + }, + { + name: "keeps failed status checks", + events: []Event{ + {Kind: EventKindStatusCheck, Outcome: "failure", Body: "check1"}, + {Kind: EventKindStatusCheck, Outcome: "success", Body: "check2"}, + {Kind: EventKindComment, Actor: "user1"}, + }, + expected: 2, // failure + comment + }, + { + name: "empty events", + events: []Event{}, + expected: 0, + }, + { + name: "mixed events", + events: []Event{ + {Kind: EventKindStatusCheck, Outcome: "success", Body: "check1"}, + {Kind: EventKindStatusCheck, Outcome: "failure", Body: "check2"}, + {Kind: EventKindCheckRun, Outcome: "success", Body: "check3"}, + {Kind: EventKindCommit, Actor: "user1"}, + }, + expected: 3, // failure status + check_run + commit + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FilterEvents(tt.events) + if len(result) != tt.expected { + t.Errorf("FilterEvents() returned %d events, expected %d", len(result), tt.expected) + } + }) + } +} + +func TestUpgradeWriteAccess(t *testing.T) { + tests := []struct { + name string + events []Event + validate func(t *testing.T, events []Event) + }{ + { + name: "upgrades actors who merge PRs", + events: []Event{ + {Kind: EventKindReview, Actor: "reviewer1", WriteAccess: WriteAccessLikely}, + {Kind: EventKindPRMerged, Actor: "reviewer1"}, + {Kind: EventKindComment, Actor: "reviewer1", WriteAccess: WriteAccessLikely}, + }, + validate: func(t *testing.T, events []Event) { + // First event should be upgraded + if events[0].WriteAccess != WriteAccessDefinitely { + t.Errorf("Event 0: WriteAccess = %d, expected %d", events[0].WriteAccess, WriteAccessDefinitely) + } + // Third event should be upgraded + if events[2].WriteAccess != WriteAccessDefinitely { + t.Errorf("Event 2: WriteAccess = %d, expected %d", events[2].WriteAccess, WriteAccessDefinitely) + } + }, + }, + { + name: "upgrades actors who add labels", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", WriteAccess: WriteAccessLikely}, + {Kind: EventKindLabeled, Actor: "user1"}, + }, + validate: func(t *testing.T, events []Event) { + if events[0].WriteAccess != WriteAccessDefinitely { + t.Errorf("Event 0: WriteAccess = %d, expected %d", events[0].WriteAccess, WriteAccessDefinitely) + } + }, + }, + { + name: "upgrades actors who remove labels", + events: []Event{ + {Kind: EventKindComment, Actor: "user1", WriteAccess: WriteAccessLikely}, + {Kind: EventKindUnlabeled, Actor: "user1"}, + }, + validate: func(t *testing.T, events []Event) { + if events[0].WriteAccess != WriteAccessDefinitely { + t.Errorf("Event 0: WriteAccess = %d, expected %d", events[0].WriteAccess, WriteAccessDefinitely) + } + }, + }, + { + name: "upgrades actors who assign users", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", WriteAccess: WriteAccessLikely}, + {Kind: EventKindAssigned, Actor: "user1"}, + }, + validate: func(t *testing.T, events []Event) { + if events[0].WriteAccess != WriteAccessDefinitely { + t.Errorf("Event 0: WriteAccess = %d, expected %d", events[0].WriteAccess, WriteAccessDefinitely) + } + }, + }, + { + name: "does not upgrade non-likely write access", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", WriteAccess: WriteAccessNo}, + {Kind: EventKindLabeled, Actor: "user1"}, + }, + validate: func(t *testing.T, events []Event) { + // Should not upgrade from No to Definitely + if events[0].WriteAccess != WriteAccessNo { + t.Errorf("Event 0: WriteAccess = %d, expected %d (should not upgrade)", events[0].WriteAccess, WriteAccessNo) + } + }, + }, + { + name: "handles empty actor names", + events: []Event{ + {Kind: EventKindLabeled, Actor: ""}, + {Kind: EventKindComment, Actor: "user1", WriteAccess: WriteAccessLikely}, + }, + validate: func(t *testing.T, events []Event) { + // Should not panic and should leave events unchanged + if events[1].WriteAccess != WriteAccessLikely { + t.Errorf("Event 1: WriteAccess = %d, expected %d", events[1].WriteAccess, WriteAccessLikely) + } + }, + }, + { + name: "handles empty events slice", + events: []Event{}, + validate: func(t *testing.T, events []Event) { + // Should not panic + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + UpgradeWriteAccess(tt.events) + tt.validate(t, tt.events) + }) + } +} + +func TestCalculateCheckSummary(t *testing.T) { + now := time.Now() + earlier := now.Add(-1 * time.Hour) + + tests := []struct { + name string + events []Event + requiredChecks []string + validate func(t *testing.T, summary *CheckSummary) + }{ + { + name: "categorizes successful checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "success", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Success) != 1 { + t.Errorf("Success count = %d, expected 1", len(summary.Success)) + } + if _, ok := summary.Success["test"]; !ok { + t.Error("Expected 'test' in Success") + } + }, + }, + { + name: "categorizes failing checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "failure", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Failing) != 1 { + t.Errorf("Failing count = %d, expected 1", len(summary.Failing)) + } + }, + }, + { + name: "categorizes error as failing", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "error", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Failing) != 1 { + t.Errorf("Failing count = %d, expected 1", len(summary.Failing)) + } + }, + }, + { + name: "categorizes cancelled checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "cancelled", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Cancelled) != 1 { + t.Errorf("Cancelled count = %d, expected 1", len(summary.Cancelled)) + } + }, + }, + { + name: "categorizes pending checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test1", Outcome: "pending", Timestamp: now}, + {Kind: EventKindCheckRun, Body: "test2", Outcome: "queued", Timestamp: now}, + {Kind: EventKindCheckRun, Body: "test3", Outcome: "in_progress", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Pending) != 3 { + t.Errorf("Pending count = %d, expected 3", len(summary.Pending)) + } + }, + }, + { + name: "uses latest status for duplicate checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "pending", Timestamp: earlier}, + {Kind: EventKindCheckRun, Body: "test", Outcome: "success", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Success) != 1 { + t.Errorf("Success count = %d, expected 1", len(summary.Success)) + } + if len(summary.Pending) != 0 { + t.Errorf("Pending count = %d, expected 0", len(summary.Pending)) + } + }, + }, + { + name: "adds missing required checks as pending", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test1", Outcome: "success", Timestamp: now}, + }, + requiredChecks: []string{"test1", "test2"}, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Pending) != 1 { + t.Errorf("Pending count = %d, expected 1", len(summary.Pending)) + } + if _, ok := summary.Pending["test2"]; !ok { + t.Error("Expected 'test2' in Pending") + } + }, + }, + { + name: "ignores checks with no name", + events: []Event{ + {Kind: EventKindCheckRun, Body: "", Outcome: "success", Timestamp: now}, + {Kind: EventKindCheckRun, Body: "test", Outcome: "success", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Success) != 1 { + t.Errorf("Success count = %d, expected 1", len(summary.Success)) + } + }, + }, + { + name: "categorizes skipped checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "skipped", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Skipped) != 1 { + t.Errorf("Skipped count = %d, expected 1", len(summary.Skipped)) + } + }, + }, + { + name: "categorizes stale checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "stale", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Stale) != 1 { + t.Errorf("Stale count = %d, expected 1", len(summary.Stale)) + } + }, + }, + { + name: "categorizes neutral checks", + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "neutral", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Neutral) != 1 { + t.Errorf("Neutral count = %d, expected 1", len(summary.Neutral)) + } + }, + }, + { + name: "handles both status_check and check_run events", + events: []Event{ + {Kind: EventKindStatusCheck, Body: "check1", Outcome: "success", Timestamp: now}, + {Kind: EventKindCheckRun, Body: "check2", Outcome: "failure", Timestamp: now}, + }, + validate: func(t *testing.T, summary *CheckSummary) { + if len(summary.Success) != 1 { + t.Errorf("Success count = %d, expected 1", len(summary.Success)) + } + if len(summary.Failing) != 1 { + t.Errorf("Failing count = %d, expected 1", len(summary.Failing)) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CalculateCheckSummary(tt.events, tt.requiredChecks) + tt.validate(t, result) + }) + } +} + +func TestCalculateApprovalSummary(t *testing.T) { + tests := []struct { + name string + events []Event + validate func(t *testing.T, summary *ApprovalSummary) + }{ + { + name: "counts approvals with write access", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "approved", WriteAccess: WriteAccessDefinitely}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithWriteAccess != 1 { + t.Errorf("ApprovalsWithWriteAccess = %d, expected 1", summary.ApprovalsWithWriteAccess) + } + }, + }, + { + name: "counts approvals without write access", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "approved", WriteAccess: WriteAccessNo}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithoutWriteAccess != 1 { + t.Errorf("ApprovalsWithoutWriteAccess = %d, expected 1", summary.ApprovalsWithoutWriteAccess) + } + }, + }, + { + name: "counts approvals with unknown write access", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "approved", WriteAccess: WriteAccessLikely}, + {Kind: EventKindReview, Actor: "user2", Outcome: "approved", WriteAccess: WriteAccessUnlikely}, + {Kind: EventKindReview, Actor: "user3", Outcome: "approved", WriteAccess: WriteAccessNA}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithUnknownAccess != 3 { + t.Errorf("ApprovalsWithUnknownAccess = %d, expected 3", summary.ApprovalsWithUnknownAccess) + } + }, + }, + { + name: "counts change requests", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "changes_requested", WriteAccess: WriteAccessDefinitely}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ChangesRequested != 1 { + t.Errorf("ChangesRequested = %d, expected 1", summary.ChangesRequested) + } + }, + }, + { + name: "uses latest review from each user", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "approved", WriteAccess: WriteAccessDefinitely}, + {Kind: EventKindReview, Actor: "user1", Outcome: "changes_requested", WriteAccess: WriteAccessDefinitely}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ChangesRequested != 1 { + t.Errorf("ChangesRequested = %d, expected 1", summary.ChangesRequested) + } + if summary.ApprovalsWithWriteAccess != 0 { + t.Errorf("ApprovalsWithWriteAccess = %d, expected 0 (latest review should override)", summary.ApprovalsWithWriteAccess) + } + }, + }, + { + name: "ignores commented reviews", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "commented", WriteAccess: WriteAccessDefinitely}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithWriteAccess != 0 { + t.Errorf("ApprovalsWithWriteAccess = %d, expected 0", summary.ApprovalsWithWriteAccess) + } + if summary.ChangesRequested != 0 { + t.Errorf("ChangesRequested = %d, expected 0", summary.ChangesRequested) + } + }, + }, + { + name: "ignores non-review events", + events: []Event{ + {Kind: EventKindComment, Actor: "user1"}, + {Kind: EventKindCommit, Actor: "user2"}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithWriteAccess != 0 { + t.Errorf("ApprovalsWithWriteAccess = %d, expected 0", summary.ApprovalsWithWriteAccess) + } + }, + }, + { + name: "handles multiple reviewers", + events: []Event{ + {Kind: EventKindReview, Actor: "user1", Outcome: "approved", WriteAccess: WriteAccessDefinitely}, + {Kind: EventKindReview, Actor: "user2", Outcome: "approved", WriteAccess: WriteAccessDefinitely}, + {Kind: EventKindReview, Actor: "user3", Outcome: "changes_requested", WriteAccess: WriteAccessNo}, + }, + validate: func(t *testing.T, summary *ApprovalSummary) { + if summary.ApprovalsWithWriteAccess != 2 { + t.Errorf("ApprovalsWithWriteAccess = %d, expected 2", summary.ApprovalsWithWriteAccess) + } + if summary.ChangesRequested != 1 { + t.Errorf("ChangesRequested = %d, expected 1", summary.ChangesRequested) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CalculateApprovalSummary(tt.events) + tt.validate(t, result) + }) + } +} + +func TestCalculateParticipantAccess(t *testing.T) { + tests := []struct { + name string + events []Event + pr *PullRequest + validate func(t *testing.T, participants map[string]int) + }{ + { + name: "includes PR author", + events: []Event{}, + pr: &PullRequest{ + Author: "author1", + AuthorWriteAccess: WriteAccessDefinitely, + }, + validate: func(t *testing.T, participants map[string]int) { + if participants["author1"] != WriteAccessDefinitely { + t.Errorf("author1 access = %d, expected %d", participants["author1"], WriteAccessDefinitely) + } + }, + }, + { + name: "includes assignees", + events: []Event{}, + pr: &PullRequest{ + Author: "author1", + Assignees: []string{"assignee1", "assignee2"}, + }, + validate: func(t *testing.T, participants map[string]int) { + if participants["assignee1"] != WriteAccessNA { + t.Errorf("assignee1 access = %d, expected %d", participants["assignee1"], WriteAccessNA) + } + if participants["assignee2"] != WriteAccessNA { + t.Errorf("assignee2 access = %d, expected %d", participants["assignee2"], WriteAccessNA) + } + }, + }, + { + name: "includes reviewers", + events: []Event{}, + pr: &PullRequest{ + Author: "author1", + Reviewers: map[string]ReviewState{ + "reviewer1": ReviewStateApproved, + "reviewer2": ReviewStatePending, + }, + }, + validate: func(t *testing.T, participants map[string]int) { + if participants["reviewer1"] != WriteAccessNA { + t.Errorf("reviewer1 access = %d, expected %d", participants["reviewer1"], WriteAccessNA) + } + if participants["reviewer2"] != WriteAccessNA { + t.Errorf("reviewer2 access = %d, expected %d", participants["reviewer2"], WriteAccessNA) + } + }, + }, + { + name: "includes event actors", + events: []Event{ + {Actor: "actor1", WriteAccess: WriteAccessDefinitely}, + {Actor: "actor2", WriteAccess: WriteAccessNo}, + }, + pr: &PullRequest{Author: "author1"}, + validate: func(t *testing.T, participants map[string]int) { + if participants["actor1"] != WriteAccessDefinitely { + t.Errorf("actor1 access = %d, expected %d", participants["actor1"], WriteAccessDefinitely) + } + if participants["actor2"] != WriteAccessNo { + t.Errorf("actor2 access = %d, expected %d", participants["actor2"], WriteAccessNo) + } + }, + }, + { + name: "upgrades write access to highest level", + events: []Event{ + {Actor: "user1", WriteAccess: WriteAccessLikely}, + {Actor: "user1", WriteAccess: WriteAccessDefinitely}, + }, + pr: &PullRequest{Author: "author1"}, + validate: func(t *testing.T, participants map[string]int) { + if participants["user1"] != WriteAccessDefinitely { + t.Errorf("user1 access = %d, expected %d (should upgrade to highest)", participants["user1"], WriteAccessDefinitely) + } + }, + }, + { + name: "handles empty actor names", + events: []Event{ + {Actor: "", WriteAccess: WriteAccessDefinitely}, + {Actor: "actor1", WriteAccess: WriteAccessNo}, + }, + pr: &PullRequest{Author: "author1"}, + validate: func(t *testing.T, participants map[string]int) { + if _, ok := participants[""]; ok { + t.Error("Empty actor should not be in participants") + } + if participants["actor1"] != WriteAccessNo { + t.Errorf("actor1 access = %d, expected %d", participants["actor1"], WriteAccessNo) + } + }, + }, + { + name: "handles empty assignee names", + events: []Event{}, + pr: &PullRequest{ + Author: "author1", + Assignees: []string{"", "assignee1"}, + }, + validate: func(t *testing.T, participants map[string]int) { + if _, ok := participants[""]; ok { + t.Error("Empty assignee should not be in participants") + } + if participants["assignee1"] != WriteAccessNA { + t.Errorf("assignee1 access = %d, expected %d", participants["assignee1"], WriteAccessNA) + } + }, + }, + { + name: "does not downgrade author's write access", + events: []Event{ + {Actor: "author1", WriteAccess: WriteAccessNo}, + }, + pr: &PullRequest{ + Author: "author1", + AuthorWriteAccess: WriteAccessDefinitely, + }, + validate: func(t *testing.T, participants map[string]int) { + if participants["author1"] != WriteAccessDefinitely { + t.Errorf("author1 access = %d, expected %d (should keep author's higher access)", participants["author1"], WriteAccessDefinitely) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CalculateParticipantAccess(tt.events, tt.pr) + tt.validate(t, result) + }) + } +} diff --git a/pkg/prx/pullrequest.go b/pkg/prx/types/types.go similarity index 50% rename from pkg/prx/pullrequest.go rename to pkg/prx/types/types.go index 172fb5b..965dfeb 100644 --- a/pkg/prx/pullrequest.go +++ b/pkg/prx/types/types.go @@ -1,9 +1,23 @@ -package prx +// Package types contains the core types and interfaces used across the prx library. +// This package is imported by platform implementations to avoid circular dependencies. +package types import ( + "context" "time" ) +// Platform fetches pull request data from a code hosting service. +// Each platform (GitHub, GitLab, Codeberg) implements its own fetching strategy. +type Platform interface { + // FetchPR retrieves a pull request with all events and metadata. + // The refTime parameter is used for cache validation decisions. + FetchPR(ctx context.Context, owner, repo string, number int, refTime time.Time) (*PullRequestData, error) + + // Name returns the platform identifier (e.g., "github", "gitlab", "codeberg"). + Name() string +} + // TestState represents the overall testing status of a pull request. const ( TestStateNone = "" // No tests or unknown state @@ -99,6 +113,107 @@ type PullRequestData struct { PullRequest PullRequest `json:"pull_request"` } +// Event kind constants for PR timeline events. +const ( + EventKindCommit = "commit" // EventKindCommit represents a commit event. + EventKindComment = "comment" // EventKindComment represents a comment event. + EventKindReview = "review" // EventKindReview represents a review event. + EventKindReviewComment = "review_comment" // EventKindReviewComment represents a review comment event. + + EventKindLabeled = "labeled" // EventKindLabeled represents a label added event. + EventKindUnlabeled = "unlabeled" // EventKindUnlabeled represents a label removed event. + + EventKindAssigned = "assigned" // EventKindAssigned represents an assignment event. + EventKindUnassigned = "unassigned" // EventKindUnassigned represents an unassignment event. + + EventKindMilestoned = "milestoned" // EventKindMilestoned represents a milestone added event. + EventKindDemilestoned = "demilestoned" // EventKindDemilestoned represents a milestone removed event. + + EventKindReviewRequested = "review_requested" // EventKindReviewRequested represents a review request event. + EventKindReviewRequestRemoved = "review_request_removed" // EventKindReviewRequestRemoved represents a review request removed event. + + EventKindPROpened = "pr_opened" // EventKindPROpened represents a PR opened event. + EventKindPRClosed = "pr_closed" // EventKindPRClosed represents a PR closed event. + EventKindPRMerged = "pr_merged" // EventKindPRMerged represents a PR merge event. + EventKindMerged = "merged" // EventKindMerged represents a merge event from timeline. + EventKindReadyForReview = "ready_for_review" // EventKindReadyForReview represents a ready for review event. + EventKindConvertToDraft = "convert_to_draft" // EventKindConvertToDraft represents a convert to draft event. + EventKindClosed = "closed" // EventKindClosed represents a PR closed event. + EventKindReopened = "reopened" // EventKindReopened represents a PR reopened event. + EventKindRenamedTitle = "renamed_title" // EventKindRenamedTitle represents a title rename event. + + EventKindMentioned = "mentioned" // EventKindMentioned represents a mention event. + EventKindReferenced = "referenced" // EventKindReferenced represents a reference event. + EventKindCrossReferenced = "cross_referenced" // EventKindCrossReferenced represents a cross-reference event. + + EventKindPinned = "pinned" // EventKindPinned represents a pin event. + EventKindUnpinned = "unpinned" // EventKindUnpinned represents an unpin event. + EventKindTransferred = "transferred" // EventKindTransferred represents a transfer event. + + EventKindSubscribed = "subscribed" // EventKindSubscribed represents a subscription event. + EventKindUnsubscribed = "unsubscribed" // EventKindUnsubscribed represents an unsubscription event. + + EventKindHeadRefDeleted = "head_ref_deleted" // EventKindHeadRefDeleted represents a head ref deletion event. + EventKindHeadRefRestored = "head_ref_restored" // EventKindHeadRefRestored represents a head ref restoration event. + EventKindHeadRefForcePushed = "head_ref_force_pushed" // EventKindHeadRefForcePushed represents a head ref force push event. + + EventKindBaseRefChanged = "base_ref_changed" // EventKindBaseRefChanged represents a base ref change event. + EventKindBaseRefForcePushed = "base_ref_force_pushed" // EventKindBaseRefForcePushed represents a base ref force push event. + + EventKindReviewDismissed = "review_dismissed" // EventKindReviewDismissed represents a review dismissed event. + + EventKindLocked = "locked" // EventKindLocked represents a lock event. + EventKindUnlocked = "unlocked" // EventKindUnlocked represents an unlock event. + + EventKindAutoMergeEnabled = "auto_merge_enabled" // EventKindAutoMergeEnabled represents an auto merge enabled event. + EventKindAutoMergeDisabled = "auto_merge_disabled" // EventKindAutoMergeDisabled represents an auto merge disabled event. + EventKindAddedToMergeQueue = "added_to_merge_queue" // EventKindAddedToMergeQueue represents an added to merge queue event. + EventKindRemovedFromMergeQueue = "removed_from_merge_queue" // EventKindRemovedFromMergeQueue represents removal from merge queue. + + // EventKindAutomaticBaseChangeSucceeded represents a successful base change. + EventKindAutomaticBaseChangeSucceeded = "automatic_base_change_succeeded" + // EventKindAutomaticBaseChangeFailed represents a failed base change. + EventKindAutomaticBaseChangeFailed = "automatic_base_change_failed" + + EventKindDeployed = "deployed" // EventKindDeployed represents a deployment event. + // EventKindDeploymentEnvironmentChanged represents a deployment environment change event. + EventKindDeploymentEnvironmentChanged = "deployment_environment_changed" + + EventKindConnected = "connected" // EventKindConnected represents a connected event. + EventKindDisconnected = "disconnected" // EventKindDisconnected represents a disconnected event. + EventKindUserBlocked = "user_blocked" // EventKindUserBlocked represents a user blocked event. + + EventKindStatusCheck = "status_check" // EventKindStatusCheck represents a status check event (from APIs). + EventKindCheckRun = "check_run" // EventKindCheckRun represents a check run event (from APIs). +) + +// WriteAccess constants for the Event.WriteAccess field. +const ( + WriteAccessNo = -2 // User confirmed to not have write access + WriteAccessUnlikely = -1 // User unlikely to have write access (CONTRIBUTOR, NONE, etc.) + WriteAccessNA = 0 // Not applicable/not set (omitted from JSON) + WriteAccessLikely = 1 // User likely has write access but unable to confirm (MEMBER with 403 API response) + WriteAccessDefinitely = 2 // User definitely has write access (OWNER, COLLABORATOR, or confirmed via API) +) + +// Event represents a single event that occurred on a pull request. +// Each event captures who did what and when, with additional context depending on the event type. +type Event struct { + Timestamp time.Time `json:"timestamp"` + Kind string `json:"kind"` + Actor string `json:"actor"` + Target string `json:"target,omitempty"` + Outcome string `json:"outcome,omitempty"` + Body string `json:"body,omitempty"` + Description string `json:"description,omitempty"` + WriteAccess int `json:"write_access,omitempty"` + Bot bool `json:"bot,omitempty"` + TargetIsBot bool `json:"target_is_bot,omitempty"` + Question bool `json:"question,omitempty"` + Required bool `json:"required,omitempty"` + Outdated bool `json:"outdated,omitempty"` // For review comments: indicates comment is on outdated code +} + // FinalizePullRequest applies final calculations and consistency fixes. func FinalizePullRequest(pullRequest *PullRequest, events []Event, requiredChecks []string, testStateFromAPI string) { pullRequest.TestState = testStateFromAPI @@ -114,7 +229,7 @@ func FinalizePullRequest(pullRequest *PullRequest, events []Event, requiredCheck pullRequest.Mergeable = &falseVal } - setMergeableDescription(pullRequest) + SetMergeableDescription(pullRequest) } // FixTestState ensures test_state is consistent with check_summary. @@ -139,11 +254,11 @@ func FixTestState(pullRequest *PullRequest) { } } -// setMergeableDescription adds human-readable description for mergeable state. -func setMergeableDescription(pullRequest *PullRequest) { +// SetMergeableDescription adds human-readable description for mergeable state. +func SetMergeableDescription(pullRequest *PullRequest) { switch pullRequest.MergeableState { case "blocked": - setBlockedDescription(pullRequest) + SetBlockedDescription(pullRequest) case "dirty": pullRequest.MergeableStateDescription = "PR has merge conflicts that need to be resolved" case "unstable": @@ -159,8 +274,8 @@ func setMergeableDescription(pullRequest *PullRequest) { } } -// setBlockedDescription determines what's blocking the PR and sets appropriate description. -func setBlockedDescription(pullRequest *PullRequest) { +// SetBlockedDescription determines what's blocking the PR and sets appropriate description. +func SetBlockedDescription(pullRequest *PullRequest) { hasApprovals := pullRequest.ApprovalSummary.ApprovalsWithWriteAccess > 0 hasFailingChecks := len(pullRequest.CheckSummary.Failing) > 0 || len(pullRequest.CheckSummary.Cancelled) > 0 hasPendingChecks := len(pullRequest.CheckSummary.Pending) > 0 diff --git a/pkg/prx/types/types_test.go b/pkg/prx/types/types_test.go new file mode 100644 index 0000000..53914ea --- /dev/null +++ b/pkg/prx/types/types_test.go @@ -0,0 +1,487 @@ +package types + +import ( + "testing" + "time" +) + +func TestFinalizePullRequest(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + pr *PullRequest + events []Event + requiredChecks []string + testStateFromAPI string + validate func(t *testing.T, pr *PullRequest) + }{ + { + name: "sets test state from API", + pr: &PullRequest{}, + events: []Event{ + {Kind: EventKindCheckRun, Body: "test", Outcome: "success", Timestamp: now}, + }, + testStateFromAPI: TestStatePassing, + validate: func(t *testing.T, pr *PullRequest) { + if pr.TestState != TestStatePassing { + t.Errorf("TestState = %q, expected %q", pr.TestState, TestStatePassing) + } + if pr.CheckSummary == nil { + t.Fatal("CheckSummary is nil") + } + if pr.ApprovalSummary == nil { + t.Fatal("ApprovalSummary is nil") + } + if pr.ParticipantAccess == nil { + t.Fatal("ParticipantAccess is nil") + } + }, + }, + { + name: "sets mergeable to false for blocked state", + pr: &PullRequest{ + MergeableState: "blocked", + }, + events: []Event{}, + validate: func(t *testing.T, pr *PullRequest) { + if pr.Mergeable == nil { + t.Fatal("Mergeable is nil") + } + if *pr.Mergeable != false { + t.Errorf("Mergeable = %v, expected false", *pr.Mergeable) + } + }, + }, + { + name: "sets mergeable to false for dirty state", + pr: &PullRequest{ + MergeableState: "dirty", + }, + events: []Event{}, + validate: func(t *testing.T, pr *PullRequest) { + if pr.Mergeable == nil { + t.Fatal("Mergeable is nil") + } + if *pr.Mergeable != false { + t.Errorf("Mergeable = %v, expected false", *pr.Mergeable) + } + }, + }, + { + name: "sets mergeable to false for unstable state", + pr: &PullRequest{ + MergeableState: "unstable", + }, + events: []Event{}, + validate: func(t *testing.T, pr *PullRequest) { + if pr.Mergeable == nil { + t.Fatal("Mergeable is nil") + } + if *pr.Mergeable != false { + t.Errorf("Mergeable = %v, expected false", *pr.Mergeable) + } + }, + }, + { + name: "sets mergeable description", + pr: &PullRequest{ + MergeableState: "clean", + }, + events: []Event{}, + validate: func(t *testing.T, pr *PullRequest) { + if pr.MergeableStateDescription == "" { + t.Error("MergeableStateDescription is empty") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + FinalizePullRequest(tt.pr, tt.events, tt.requiredChecks, tt.testStateFromAPI) + tt.validate(t, tt.pr) + }) + } +} + +func TestFixTestState(t *testing.T) { + tests := []struct { + name string + pr *PullRequest + expected string + }{ + { + name: "sets failing when checks fail", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Failing: map[string]string{"test": "failed"}, + }, + }, + expected: TestStateFailing, + }, + { + name: "sets failing when checks cancelled", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Cancelled: map[string]string{"test": "cancelled"}, + }, + }, + expected: TestStateFailing, + }, + { + name: "sets pending when checks pending", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Pending: map[string]string{"test": "pending"}, + }, + }, + expected: TestStatePending, + }, + { + name: "sets passing when checks succeed", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Success: map[string]string{"test": "passed"}, + }, + }, + expected: TestStatePassing, + }, + { + name: "prefers failing over pending", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Failing: map[string]string{"test1": "failed"}, + Pending: map[string]string{"test2": "pending"}, + }, + }, + expected: TestStateFailing, + }, + { + name: "prefers pending over passing", + pr: &PullRequest{ + CheckSummary: &CheckSummary{ + Success: map[string]string{"test1": "passed"}, + Pending: map[string]string{"test2": "pending"}, + }, + }, + expected: TestStatePending, + }, + { + name: "preserves existing test state when no checks", + pr: &PullRequest{ + TestState: TestStatePassing, + CheckSummary: &CheckSummary{}, + }, + expected: TestStatePassing, + }, + { + name: "sets none when no checks and no existing state", + pr: &PullRequest{ + CheckSummary: &CheckSummary{}, + }, + expected: TestStateNone, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + FixTestState(tt.pr) + if tt.pr.TestState != tt.expected { + t.Errorf("TestState = %q, expected %q", tt.pr.TestState, tt.expected) + } + }) + } +} + +func TestSetMergeableDescription(t *testing.T) { + tests := []struct { + name string + mergeableState string + checkSummary *CheckSummary + approvalSummary *ApprovalSummary + expectedDescription string + descriptionShouldBe string // for more flexible matching + shouldNotBeEmpty bool + }{ + { + name: "clean state", + mergeableState: "clean", + expectedDescription: "PR is ready to merge", + }, + { + name: "dirty state", + mergeableState: "dirty", + expectedDescription: "PR has merge conflicts that need to be resolved", + }, + { + name: "unstable state", + mergeableState: "unstable", + expectedDescription: "PR is mergeable but status checks are failing", + }, + { + name: "unknown state", + mergeableState: "unknown", + expectedDescription: "Merge status is being calculated", + }, + { + name: "draft state", + mergeableState: "draft", + expectedDescription: "PR is in draft state", + }, + { + name: "blocked state triggers SetBlockedDescription", + mergeableState: "blocked", + checkSummary: &CheckSummary{}, + approvalSummary: &ApprovalSummary{}, + shouldNotBeEmpty: true, + }, + { + name: "unknown/unhandled state", + mergeableState: "some_other_state", + expectedDescription: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := &PullRequest{ + MergeableState: tt.mergeableState, + CheckSummary: tt.checkSummary, + ApprovalSummary: tt.approvalSummary, + } + if pr.CheckSummary == nil { + pr.CheckSummary = &CheckSummary{} + } + if pr.ApprovalSummary == nil { + pr.ApprovalSummary = &ApprovalSummary{} + } + + SetMergeableDescription(pr) + + if tt.shouldNotBeEmpty { + if pr.MergeableStateDescription == "" { + t.Error("MergeableStateDescription should not be empty") + } + } else if tt.expectedDescription != "" { + if pr.MergeableStateDescription != tt.expectedDescription { + t.Errorf("MergeableStateDescription = %q, expected %q", pr.MergeableStateDescription, tt.expectedDescription) + } + } + }) + } +} + +func TestSetBlockedDescription(t *testing.T) { + tests := []struct { + name string + approvalSummary *ApprovalSummary + checkSummary *CheckSummary + expectedDescription string + }{ + { + name: "requires approval only", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 0, + }, + checkSummary: &CheckSummary{ + Failing: map[string]string{}, + Pending: map[string]string{}, + }, + expectedDescription: "PR requires approval", + }, + { + name: "requires approval with pending checks", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 0, + }, + checkSummary: &CheckSummary{ + Failing: map[string]string{}, + Pending: map[string]string{"test": "pending"}, + }, + expectedDescription: "PR requires approval and has pending status checks", + }, + { + name: "has failing checks without approval", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 0, + }, + checkSummary: &CheckSummary{ + Failing: map[string]string{"test": "failed"}, + }, + expectedDescription: "PR has failing status checks and requires approval", + }, + { + name: "has failing checks with approval", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 1, + }, + checkSummary: &CheckSummary{ + Failing: map[string]string{"test": "failed"}, + }, + expectedDescription: "PR is blocked by failing status checks", + }, + { + name: "has pending checks with approval", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 1, + }, + checkSummary: &CheckSummary{ + Pending: map[string]string{"test": "pending"}, + Failing: map[string]string{}, + }, + expectedDescription: "PR is blocked by pending status checks", + }, + { + name: "has cancelled checks (treated as failing)", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 0, + }, + checkSummary: &CheckSummary{ + Cancelled: map[string]string{"test": "cancelled"}, + }, + expectedDescription: "PR has failing status checks and requires approval", + }, + { + name: "generic blocked message", + approvalSummary: &ApprovalSummary{ + ApprovalsWithWriteAccess: 1, + }, + checkSummary: &CheckSummary{ + Failing: map[string]string{}, + Pending: map[string]string{}, + }, + expectedDescription: "PR is blocked by required status checks, reviews, or branch protection rules", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := &PullRequest{ + ApprovalSummary: tt.approvalSummary, + CheckSummary: tt.checkSummary, + } + + SetBlockedDescription(pr) + + if pr.MergeableStateDescription != tt.expectedDescription { + t.Errorf("MergeableStateDescription = %q, expected %q", pr.MergeableStateDescription, tt.expectedDescription) + } + }) + } +} + +func TestReviewStateConstants(t *testing.T) { + // Test that constants are properly defined + tests := []struct { + name string + state ReviewState + value string + }{ + {"pending", ReviewStatePending, "pending"}, + {"approved", ReviewStateApproved, "approved"}, + {"changes_requested", ReviewStateChangesRequested, "changes_requested"}, + {"commented", ReviewStateCommented, "commented"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if string(tt.state) != tt.value { + t.Errorf("ReviewState %q = %q, expected %q", tt.name, tt.state, tt.value) + } + }) + } +} + +func TestTestStateConstants(t *testing.T) { + // Test that constants are properly defined + tests := []struct { + name string + state string + value string + }{ + {"none", TestStateNone, ""}, + {"queued", TestStateQueued, "queued"}, + {"running", TestStateRunning, "running"}, + {"passing", TestStatePassing, "passing"}, + {"failing", TestStateFailing, "failing"}, + {"pending", TestStatePending, "pending"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.state != tt.value { + t.Errorf("TestState %q = %q, expected %q", tt.name, tt.state, tt.value) + } + }) + } +} + +func TestWriteAccessConstants(t *testing.T) { + // Test that constants have expected values + tests := []struct { + name string + value int + want int + }{ + {"WriteAccessNo", WriteAccessNo, -2}, + {"WriteAccessUnlikely", WriteAccessUnlikely, -1}, + {"WriteAccessNA", WriteAccessNA, 0}, + {"WriteAccessLikely", WriteAccessLikely, 1}, + {"WriteAccessDefinitely", WriteAccessDefinitely, 2}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.value != tt.want { + t.Errorf("%s = %d, expected %d", tt.name, tt.value, tt.want) + } + }) + } +} + +func TestEventKindConstants(t *testing.T) { + // Just verify some key event kinds are defined correctly + tests := []struct { + name string + kind string + value string + }{ + {"commit", EventKindCommit, "commit"}, + {"comment", EventKindComment, "comment"}, + {"review", EventKindReview, "review"}, + {"status_check", EventKindStatusCheck, "status_check"}, + {"check_run", EventKindCheckRun, "check_run"}, + {"labeled", EventKindLabeled, "labeled"}, + {"pr_merged", EventKindPRMerged, "pr_merged"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.kind != tt.value { + t.Errorf("EventKind %q = %q, expected %q", tt.name, tt.kind, tt.value) + } + }) + } +} + +func TestPlatformConstants(t *testing.T) { + // Test platform constants from url.go (but they're used by types) + tests := []struct { + name string + value string + want string + }{ + {"GitHub", PlatformGitHub, "github"}, + {"GitLab", PlatformGitLab, "gitlab"}, + {"Codeberg", PlatformCodeberg, "codeberg"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.value != tt.want { + t.Errorf("%s = %q, expected %q", tt.name, tt.value, tt.want) + } + }) + } +} diff --git a/pkg/prx/url.go b/pkg/prx/types/url.go similarity index 98% rename from pkg/prx/url.go rename to pkg/prx/types/url.go index b7abb1a..3e3c072 100644 --- a/pkg/prx/url.go +++ b/pkg/prx/types/url.go @@ -1,4 +1,4 @@ -package prx +package types import ( "errors" @@ -111,7 +111,7 @@ func ParseURL(input string) (*ParsedURL, error) { host := match[1] // Detect platform from host if possible - platform := detectPlatformFromHost(host) + platform := DetectPlatformFromHost(host) return &ParsedURL{ Platform: platform, @@ -136,8 +136,8 @@ func ParseURL(input string) (*ParsedURL, error) { return nil, errors.New("unrecognized URL format") } -// detectPlatformFromHost detects platform from hostname, defaulting to Gitea for unknown hosts. -func detectPlatformFromHost(host string) string { +// DetectPlatformFromHost detects platform from hostname, defaulting to Gitea for unknown hosts. +func DetectPlatformFromHost(host string) string { host = strings.ToLower(host) switch { diff --git a/pkg/prx/types/url_test.go b/pkg/prx/types/url_test.go new file mode 100644 index 0000000..5a7e8e9 --- /dev/null +++ b/pkg/prx/types/url_test.go @@ -0,0 +1,667 @@ +package types + +import ( + "testing" +) + +func TestParseURL(t *testing.T) { + tests := []struct { + name string + input string + expected *ParsedURL + expectError bool + }{ + // GitHub URLs + { + name: "github PR with https", + input: "https://github.com/owner/repo/pull/123", + expected: &ParsedURL{ + Platform: PlatformGitHub, + Host: "github.com", + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + { + name: "github PR without https", + input: "github.com/owner/repo/pull/456", + expected: &ParsedURL{ + Platform: PlatformGitHub, + Host: "github.com", + Owner: "owner", + Repo: "repo", + Number: 456, + }, + }, + { + name: "github PR with dashes and dots", + input: "https://github.com/org-name/my.repo/pull/789", + expected: &ParsedURL{ + Platform: PlatformGitHub, + Host: "github.com", + Owner: "org-name", + Repo: "my.repo", + Number: 789, + }, + }, + { + name: "github PR with fragment", + input: "https://github.com/owner/repo/pull/123#issuecomment-123456", + expected: &ParsedURL{ + Platform: PlatformGitHub, + Host: "github.com", + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + { + name: "github PR with query params", + input: "https://github.com/owner/repo/pull/123?foo=bar", + expected: &ParsedURL{ + Platform: PlatformGitHub, + Host: "github.com", + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + // GitLab URLs + { + name: "gitlab MR with https", + input: "https://gitlab.com/owner/repo/-/merge_requests/123", + expected: &ParsedURL{ + Platform: PlatformGitLab, + Host: "gitlab.com", + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + { + name: "gitlab self-hosted MR", + input: "https://gitlab.example.com/owner/repo/-/merge_requests/456", + expected: &ParsedURL{ + Platform: PlatformGitLab, + Host: "gitlab.example.com", + Owner: "owner", + Repo: "repo", + Number: 456, + }, + }, + // Codeberg URLs + { + name: "codeberg PR with https", + input: "https://codeberg.org/owner/repo/pulls/123", + expected: &ParsedURL{ + Platform: PlatformCodeberg, + Host: "codeberg.org", + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + { + name: "codeberg PR without https", + input: "codeberg.org/owner/repo/pulls/456", + expected: &ParsedURL{ + Platform: PlatformCodeberg, + Host: "codeberg.org", + Owner: "owner", + Repo: "repo", + Number: 456, + }, + }, + // Generic Gitea URLs + { + name: "gitea instance PR", + input: "https://gitea.example.com/owner/repo/pulls/789", + expected: &ParsedURL{ + Platform: "gitea", + Host: "gitea.example.com", + Owner: "owner", + Repo: "repo", + Number: 789, + }, + }, + // Error cases + { + name: "empty URL", + input: "", + expectError: true, + }, + { + name: "whitespace only", + input: " ", + expectError: true, + }, + { + name: "invalid github URL", + input: "https://github.com/owner/repo", + expectError: true, + }, + { + name: "invalid gitlab URL", + input: "https://gitlab.com/owner/repo", + expectError: true, + }, + { + name: "unrecognized format", + input: "https://example.com/something", + expectError: true, + }, + { + name: "invalid PR number", + input: "https://github.com/owner/repo/pull/abc", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseURL(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("ParseURL(%q) expected error, got nil", tt.input) + } + return + } + + if err != nil { + t.Errorf("ParseURL(%q) unexpected error: %v", tt.input, err) + return + } + + if result.Platform != tt.expected.Platform { + t.Errorf("Platform = %q, expected %q", result.Platform, tt.expected.Platform) + } + if result.Host != tt.expected.Host { + t.Errorf("Host = %q, expected %q", result.Host, tt.expected.Host) + } + if result.Owner != tt.expected.Owner { + t.Errorf("Owner = %q, expected %q", result.Owner, tt.expected.Owner) + } + if result.Repo != tt.expected.Repo { + t.Errorf("Repo = %q, expected %q", result.Repo, tt.expected.Repo) + } + if result.Number != tt.expected.Number { + t.Errorf("Number = %d, expected %d", result.Number, tt.expected.Number) + } + }) + } +} + +func TestDetectPlatformFromHost(t *testing.T) { + tests := []struct { + name string + host string + expected string + }{ + {name: "github.com", host: "github.com", expected: PlatformGitHub}, + {name: "GitHub subdomain", host: "api.github.com", expected: PlatformGitHub}, + {name: "gitlab.com", host: "gitlab.com", expected: PlatformGitLab}, + {name: "GitLab self-hosted", host: "gitlab.example.com", expected: PlatformGitLab}, + {name: "codeberg.org", host: "codeberg.org", expected: PlatformCodeberg}, + {name: "unknown host defaults to gitea", host: "git.example.com", expected: "gitea"}, + {name: "uppercase GitHub", host: "GITHUB.COM", expected: PlatformGitHub}, + {name: "uppercase GitLab", host: "GITLAB.COM", expected: PlatformGitLab}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := DetectPlatformFromHost(tt.host) + if result != tt.expected { + t.Errorf("DetectPlatformFromHost(%q) = %q, expected %q", tt.host, result, tt.expected) + } + }) + } +} + +func TestDetectPlatform(t *testing.T) { + tests := []struct { + name string + host string + expected string + }{ + {name: "github.com", host: "github.com", expected: PlatformGitHub}, + {name: "GitHub subdomain", host: "api.github.com", expected: PlatformGitHub}, + {name: "gitlab.com", host: "gitlab.com", expected: PlatformGitLab}, + {name: "GitLab self-hosted", host: "gitlab.example.com", expected: PlatformGitLab}, + {name: "codeberg.org", host: "codeberg.org", expected: PlatformCodeberg}, + {name: "unknown host returns empty", host: "git.example.com", expected: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := DetectPlatform(tt.host) + if result != tt.expected { + t.Errorf("DetectPlatform(%q) = %q, expected %q", tt.host, result, tt.expected) + } + }) + } +} + +func TestBuildGitHubURL(t *testing.T) { + tests := []struct { + name string + owner string + repo string + number int + expected string + }{ + { + name: "basic URL", + owner: "owner", + repo: "repo", + number: 123, + expected: "https://github.com/owner/repo/pull/123", + }, + { + name: "with dashes and dots", + owner: "my-org", + repo: "my.repo", + number: 456, + expected: "https://github.com/my-org/my.repo/pull/456", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := BuildGitHubURL(tt.owner, tt.repo, tt.number) + if result != tt.expected { + t.Errorf("BuildGitHubURL() = %q, expected %q", result, tt.expected) + } + }) + } +} + +func TestBuildGitLabURL(t *testing.T) { + tests := []struct { + name string + host string + owner string + repo string + number int + expected string + }{ + { + name: "default host", + host: "", + owner: "owner", + repo: "repo", + number: 123, + expected: "https://gitlab.com/owner/repo/-/merge_requests/123", + }, + { + name: "gitlab.com explicit", + host: "gitlab.com", + owner: "owner", + repo: "repo", + number: 456, + expected: "https://gitlab.com/owner/repo/-/merge_requests/456", + }, + { + name: "self-hosted", + host: "gitlab.example.com", + owner: "owner", + repo: "repo", + number: 789, + expected: "https://gitlab.example.com/owner/repo/-/merge_requests/789", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := BuildGitLabURL(tt.host, tt.owner, tt.repo, tt.number) + if result != tt.expected { + t.Errorf("BuildGitLabURL() = %q, expected %q", result, tt.expected) + } + }) + } +} + +func TestBuildCodebergURL(t *testing.T) { + result := BuildCodebergURL("owner", "repo", 123) + expected := "https://codeberg.org/owner/repo/pulls/123" + if result != expected { + t.Errorf("BuildCodebergURL() = %q, expected %q", result, expected) + } +} + +func TestBuildGiteaURL(t *testing.T) { + tests := []struct { + name string + host string + owner string + repo string + number int + expected string + }{ + { + name: "default host", + host: "", + owner: "owner", + repo: "repo", + number: 123, + expected: "https://codeberg.org/owner/repo/pulls/123", + }, + { + name: "custom host", + host: "gitea.example.com", + owner: "owner", + repo: "repo", + number: 456, + expected: "https://gitea.example.com/owner/repo/pulls/456", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := BuildGiteaURL(tt.host, tt.owner, tt.repo, tt.number) + if result != tt.expected { + t.Errorf("BuildGiteaURL() = %q, expected %q", result, tt.expected) + } + }) + } +} + +func TestNormalizeURL(t *testing.T) { + tests := []struct { + name string + input string + expected string + expectError bool + }{ + { + name: "github URL", + input: "github.com/owner/repo/pull/123", + expected: "https://github.com/owner/repo/pull/123", + }, + { + name: "github URL with fragment", + input: "https://github.com/owner/repo/pull/123#issuecomment-456", + expected: "https://github.com/owner/repo/pull/123", + }, + { + name: "gitlab URL", + input: "https://gitlab.com/owner/repo/-/merge_requests/123", + expected: "https://gitlab.com/owner/repo/-/merge_requests/123", + }, + { + name: "codeberg URL", + input: "codeberg.org/owner/repo/pulls/123", + expected: "https://codeberg.org/owner/repo/pulls/123", + }, + { + name: "invalid URL", + input: "not-a-url", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := NormalizeURL(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("NormalizeURL(%q) expected error, got nil", tt.input) + } + return + } + + if err != nil { + t.Errorf("NormalizeURL(%q) unexpected error: %v", tt.input, err) + return + } + + if result != tt.expected { + t.Errorf("NormalizeURL(%q) = %q, expected %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestIsValidPRURL(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + {name: "valid github", input: "https://github.com/owner/repo/pull/123", expected: true}, + {name: "valid gitlab", input: "https://gitlab.com/owner/repo/-/merge_requests/123", expected: true}, + {name: "valid codeberg", input: "https://codeberg.org/owner/repo/pulls/123", expected: true}, + {name: "invalid format", input: "not-a-url", expected: false}, + {name: "empty string", input: "", expected: false}, + {name: "incomplete github", input: "https://github.com/owner/repo", expected: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsValidPRURL(tt.input) + if result != tt.expected { + t.Errorf("IsValidPRURL(%q) = %v, expected %v", tt.input, result, tt.expected) + } + }) + } +} + +func TestExtractShortRef(t *testing.T) { + tests := []struct { + name string + input string + expected string + expectError bool + }{ + { + name: "github URL", + input: "https://github.com/owner/repo/pull/123", + expected: "owner/repo#123", + }, + { + name: "gitlab URL", + input: "https://gitlab.com/owner/repo/-/merge_requests/456", + expected: "owner/repo#456", + }, + { + name: "codeberg URL", + input: "https://codeberg.org/owner/repo/pulls/789", + expected: "owner/repo#789", + }, + { + name: "invalid URL", + input: "not-a-url", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ExtractShortRef(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("ExtractShortRef(%q) expected error, got nil", tt.input) + } + return + } + + if err != nil { + t.Errorf("ExtractShortRef(%q) unexpected error: %v", tt.input, err) + return + } + + if result != tt.expected { + t.Errorf("ExtractShortRef(%q) = %q, expected %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestParseShortRef(t *testing.T) { + tests := []struct { + name string + input string + expected *ShortRef + expectError bool + }{ + { + name: "hash format", + input: "owner/repo#123", + expected: &ShortRef{ + Owner: "owner", + Repo: "repo", + Number: 123, + }, + }, + { + name: "slash format", + input: "owner/repo/456", + expected: &ShortRef{ + Owner: "owner", + Repo: "repo", + Number: 456, + }, + }, + { + name: "with whitespace", + input: " owner/repo#789 ", + expected: &ShortRef{ + Owner: "owner", + Repo: "repo", + Number: 789, + }, + }, + { + name: "invalid format - too few parts", + input: "owner#123", + expectError: true, + }, + { + name: "invalid format - too many parts", + input: "owner/repo/extra#123", + expectError: true, + }, + { + name: "invalid number", + input: "owner/repo#abc", + expectError: true, + }, + { + name: "empty string", + input: "", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseShortRef(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("ParseShortRef(%q) expected error, got nil", tt.input) + } + return + } + + if err != nil { + t.Errorf("ParseShortRef(%q) unexpected error: %v", tt.input, err) + return + } + + if result.Owner != tt.expected.Owner { + t.Errorf("Owner = %q, expected %q", result.Owner, tt.expected.Owner) + } + if result.Repo != tt.expected.Repo { + t.Errorf("Repo = %q, expected %q", result.Repo, tt.expected.Repo) + } + if result.Number != tt.expected.Number { + t.Errorf("Number = %d, expected %d", result.Number, tt.expected.Number) + } + }) + } +} + +func TestParseOwnerRepoPR(t *testing.T) { + tests := []struct { + name string + input string + expected *ParsedPR + expectError bool + }{ + { + name: "full github URL", + input: "https://github.com/owner/repo/pull/123", + expected: &ParsedPR{ + Owner: "owner", + Repo: "repo", + Number: 123, + Platform: PlatformGitHub, + }, + }, + { + name: "full gitlab URL", + input: "https://gitlab.com/owner/repo/-/merge_requests/456", + expected: &ParsedPR{ + Owner: "owner", + Repo: "repo", + Number: 456, + Platform: PlatformGitLab, + }, + }, + { + name: "short ref with hash", + input: "owner/repo#789", + expected: &ParsedPR{ + Owner: "owner", + Repo: "repo", + Number: 789, + Platform: "", + }, + }, + { + name: "short ref with slash", + input: "owner/repo/101", + expected: &ParsedPR{ + Owner: "owner", + Repo: "repo", + Number: 101, + Platform: "", + }, + }, + { + name: "invalid format", + input: "not-valid", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := ParseOwnerRepoPR(tt.input) + if tt.expectError { + if err == nil { + t.Errorf("ParseOwnerRepoPR(%q) expected error, got nil", tt.input) + } + return + } + + if err != nil { + t.Errorf("ParseOwnerRepoPR(%q) unexpected error: %v", tt.input, err) + return + } + + if result.Owner != tt.expected.Owner { + t.Errorf("Owner = %q, expected %q", result.Owner, tt.expected.Owner) + } + if result.Repo != tt.expected.Repo { + t.Errorf("Repo = %q, expected %q", result.Repo, tt.expected.Repo) + } + if result.Number != tt.expected.Number { + t.Errorf("Number = %d, expected %d", result.Number, tt.expected.Number) + } + if result.Platform != tt.expected.Platform { + t.Errorf("Platform = %q, expected %q", result.Platform, tt.expected.Platform) + } + }) + } +} diff --git a/pkg/prx/utils.go b/pkg/prx/types/utils.go similarity index 93% rename from pkg/prx/utils.go rename to pkg/prx/types/utils.go index 1a48854..d2abbe3 100644 --- a/pkg/prx/utils.go +++ b/pkg/prx/types/utils.go @@ -1,4 +1,4 @@ -package prx +package types import ( "regexp" @@ -147,17 +147,6 @@ func ContainsQuestion(text string) bool { return false } -func isHexString(s string) bool { - for i := range s { - c := s[i] - if (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F') { - continue - } - return false - } - return true -} - // Truncate returns the first maxTruncateLength characters of s. func Truncate(s string) string { if len(s) <= maxTruncateLength { diff --git a/pkg/prx/types/utils_test.go b/pkg/prx/types/utils_test.go new file mode 100644 index 0000000..e943efd --- /dev/null +++ b/pkg/prx/types/utils_test.go @@ -0,0 +1,320 @@ +package types + +import ( + "strings" + "testing" +) + +func TestContainsQuestion(t *testing.T) { + tests := []struct { + name string + text string + expected bool + }{ + // Question mark tests + { + name: "explicit question mark", + text: "What is this?", + expected: true, + }, + { + name: "question mark in middle", + text: "Really? I don't think so.", + expected: true, + }, + // Direct questions + { + name: "how can pattern", + text: "How can we fix this?", + expected: true, + }, + { + name: "how do pattern", + text: "How do you think we should proceed", + expected: true, + }, + { + name: "how would pattern", + text: "How would this work", + expected: true, + }, + { + name: "how should pattern", + text: "How should I implement this", + expected: true, + }, + { + name: "how about pattern", + text: "How about we try a different approach", + expected: true, + }, + { + name: "how to pattern", + text: "How to test this properly", + expected: true, + }, + // Modal questions + { + name: "should I pattern", + text: "Should I merge this now", + expected: true, + }, + { + name: "should we pattern", + text: "Should we wait for approval", + expected: true, + }, + { + name: "can I pattern", + text: "Can I get a review on this", + expected: true, + }, + { + name: "can you pattern", + text: "Can you take a look at this", + expected: true, + }, + { + name: "could you pattern", + text: "Could you review this code", + expected: true, + }, + { + name: "would you pattern", + text: "Would you approve this change", + expected: true, + }, + // What questions + { + name: "what do you think", + text: "What do you think about this approach", + expected: true, + }, + { + name: "what's the pattern", + text: "What's the best way to handle this", + expected: true, + }, + { + name: "what is the pattern", + text: "What is the status of the tests", + expected: true, + }, + { + name: "what about pattern", + text: "What about using a different library", + expected: true, + }, + // Request patterns + { + name: "any suggestions", + text: "Any suggestions for improving this", + expected: true, + }, + { + name: "any ideas", + text: "Any ideas on how to fix this", + expected: true, + }, + { + name: "anyone know", + text: "Anyone know why this is failing", + expected: true, + }, + { + name: "does anyone pattern", + text: "Does anyone have time to review", + expected: true, + }, + { + name: "do we pattern", + text: "Do we need to update the docs", + expected: true, + }, + // Possibility questions + { + name: "is it possible", + text: "Is it possible to merge this today", + expected: true, + }, + { + name: "is there a way", + text: "Is there a way to speed this up", + expected: true, + }, + { + name: "is this pattern", + text: "Is this ready to merge", + expected: true, + }, + // Indirect questions + { + name: "wondering if", + text: "I was wondering if this is correct", + expected: true, + }, + { + name: "thoughts on", + text: "Thoughts on this implementation", + expected: true, + }, + { + name: "need help", + text: "I need help with this error", + expected: true, + }, + // Why/when/where questions + { + name: "why is pattern", + text: "Why is this test failing", + expected: true, + }, + { + name: "when should pattern", + text: "When should we deploy this", + expected: true, + }, + { + name: "where can pattern", + text: "Where can I find the documentation", + expected: true, + }, + { + name: "who can pattern", + text: "Who can approve this PR", + expected: true, + }, + // Have/has questions + { + name: "have you pattern", + text: "Have you tested this locally", + expected: true, + }, + { + name: "has anyone pattern", + text: "Has anyone seen this issue before", + expected: true, + }, + // Non-questions + { + name: "statement with question word", + text: "I know how this works", + expected: false, + }, + { + name: "partial match should not trigger", + text: "I showed the results to the team", + expected: false, + }, + { + name: "empty string", + text: "", + expected: false, + }, + { + name: "very short text", + text: "ok", + expected: false, + }, + { + name: "statement without question patterns", + text: "This looks good to me. LGTM!", + expected: false, + }, + { + name: "imperative command", + text: "Please merge this when ready", + expected: false, + }, + // Word boundary tests + { + name: "word boundary - how in middle", + text: "I showed them the solution", + expected: false, + }, + { + name: "word boundary - can in cannot", + text: "I cannot approve this yet", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ContainsQuestion(tt.text) + if result != tt.expected { + t.Errorf("ContainsQuestion(%q) = %v, expected %v", tt.text, result, tt.expected) + } + }) + } +} + +func TestTruncate(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "short string", + input: "Hello", + expected: "Hello", + }, + { + name: "exactly max length", + input: strings.Repeat("a", maxTruncateLength), + expected: strings.Repeat("a", maxTruncateLength), + }, + { + name: "one over max length", + input: strings.Repeat("a", maxTruncateLength+1), + expected: strings.Repeat("a", maxTruncateLength), + }, + { + name: "much longer than max", + input: strings.Repeat("a", 500), + expected: strings.Repeat("a", maxTruncateLength), + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "unicode characters", + input: strings.Repeat("→", 200), // Each → is 3 bytes, so 200 chars = 600 bytes + // Truncate operates on byte length, so result will be truncated at 256 bytes + // which will cut off mid-character, but Go handles this gracefully + expected: strings.Repeat("→", 200)[:maxTruncateLength], + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := Truncate(tt.input) + if result != tt.expected { + t.Errorf("Truncate() length = %d, expected %d", len(result), len(tt.expected)) + } + if len(result) > maxTruncateLength { + t.Errorf("Truncate() returned string longer than max: %d > %d", len(result), maxTruncateLength) + } + }) + } +} + +func BenchmarkContainsQuestion(b *testing.B) { + texts := []string{ + "What is this?", + "How can we fix this issue", + "This is a statement without questions", + "Any suggestions on this approach", + "I showed them the solution", + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, text := range texts { + ContainsQuestion(text) + } + } +} diff --git a/pkg/prx/upgrade_write_access_test.go b/pkg/prx/upgrade_write_access_test.go index 10e97b2..fb94852 100644 --- a/pkg/prx/upgrade_write_access_test.go +++ b/pkg/prx/upgrade_write_access_test.go @@ -3,6 +3,8 @@ package prx import ( "testing" "time" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestUpgradeWriteAccess(t *testing.T) { @@ -10,17 +12,17 @@ func TestUpgradeWriteAccess(t *testing.T) { tests := []struct { name string - events []Event + events []types.Event expected map[string]int // actor -> expected write access }{ { name: "upgrade write access for user who merged PR", - events: []Event{ + events: []types.Event{ { Kind: "comment", Timestamp: now.Add(-2 * time.Hour), Actor: "reviewer1", - WriteAccess: WriteAccessLikely, // 1 + WriteAccess: types.WriteAccessLikely, // 1 }, { Kind: "pr_merged", @@ -29,17 +31,17 @@ func TestUpgradeWriteAccess(t *testing.T) { }, }, expected: map[string]int{ - "reviewer1": WriteAccessDefinitely, // Should be upgraded to 2 + "reviewer1": types.WriteAccessDefinitely, // Should be upgraded to 2 }, }, { name: "upgrade write access for user who labeled issue", - events: []Event{ + events: []types.Event{ { Kind: "review", Timestamp: now.Add(-3 * time.Hour), Actor: "maintainer", - WriteAccess: WriteAccessLikely, // 1 + WriteAccess: types.WriteAccessLikely, // 1 Outcome: "approved", }, { @@ -50,17 +52,17 @@ func TestUpgradeWriteAccess(t *testing.T) { }, }, expected: map[string]int{ - "maintainer": WriteAccessDefinitely, // Should be upgraded to 2 + "maintainer": types.WriteAccessDefinitely, // Should be upgraded to 2 }, }, { name: "don't upgrade if already definitely has write access", - events: []Event{ + events: []types.Event{ { Kind: "comment", Timestamp: now.Add(-2 * time.Hour), Actor: "owner", - WriteAccess: WriteAccessDefinitely, // Already 2 + WriteAccess: types.WriteAccessDefinitely, // Already 2 }, { Kind: "pr_merged", @@ -69,17 +71,17 @@ func TestUpgradeWriteAccess(t *testing.T) { }, }, expected: map[string]int{ - "owner": WriteAccessDefinitely, // Should remain 2 + "owner": types.WriteAccessDefinitely, // Should remain 2 }, }, { name: "don't upgrade if user has no write access", - events: []Event{ + events: []types.Event{ { Kind: "comment", Timestamp: now.Add(-2 * time.Hour), Actor: "contributor", - WriteAccess: WriteAccessUnlikely, // -1 + WriteAccess: types.WriteAccessUnlikely, // -1 }, { Kind: "comment", @@ -89,23 +91,23 @@ func TestUpgradeWriteAccess(t *testing.T) { }, }, expected: map[string]int{ - "contributor": WriteAccessUnlikely, // Should remain 0 + "contributor": types.WriteAccessUnlikely, // Should remain 0 }, }, { name: "upgrade multiple users based on different actions", - events: []Event{ + events: []types.Event{ { Kind: "comment", Timestamp: now.Add(-4 * time.Hour), Actor: "user1", - WriteAccess: WriteAccessLikely, // 1 + WriteAccess: types.WriteAccessLikely, // 1 }, { Kind: "review", Timestamp: now.Add(-3 * time.Hour), Actor: "user2", - WriteAccess: WriteAccessLikely, // 1 + WriteAccess: types.WriteAccessLikely, // 1 Outcome: "approved", }, { @@ -121,18 +123,18 @@ func TestUpgradeWriteAccess(t *testing.T) { }, }, expected: map[string]int{ - "user1": WriteAccessDefinitely, // Should be upgraded to 2 - "user2": WriteAccessDefinitely, // Should be upgraded to 2 + "user1": types.WriteAccessDefinitely, // Should be upgraded to 2 + "user2": types.WriteAccessDefinitely, // Should be upgraded to 2 }, }, { name: "handle events with nil write access", - events: []Event{ + events: []types.Event{ { Kind: "comment", Timestamp: now.Add(-2 * time.Hour), Actor: "user1", - WriteAccess: WriteAccessNA, // no write access info + WriteAccess: types.WriteAccessNA, // no write access info }, { Kind: "labeled", @@ -149,15 +151,15 @@ func TestUpgradeWriteAccess(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Make a copy of events to avoid modifying test data - events := make([]Event, len(tt.events)) + events := make([]types.Event, len(tt.events)) copy(events, tt.events) // Apply the upgrade function - UpgradeWriteAccess(events) + types.UpgradeWriteAccess(events) // Check results - look for events with WriteAccess field for _, event := range events { - if event.WriteAccess != WriteAccessNA { + if event.WriteAccess != types.WriteAccessNA { if expectedAccess, ok := tt.expected[event.Actor]; ok { if event.WriteAccess != expectedAccess { t.Errorf("%s: Actor %s has write access %d, expected %d", diff --git a/pkg/prx/url_test.go b/pkg/prx/url_test.go index acab67d..5349f1d 100644 --- a/pkg/prx/url_test.go +++ b/pkg/prx/url_test.go @@ -3,6 +3,8 @@ package prx import ( "strings" "testing" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) //nolint:maintidx // Table-driven test with many security test cases @@ -10,7 +12,7 @@ func TestParseURL(t *testing.T) { tests := []struct { name string input string - want *ParsedURL + want *types.ParsedURL wantErr bool errMatch string }{ @@ -18,8 +20,8 @@ func TestParseURL(t *testing.T) { { name: "github full URL with https", input: "https://github.com/owner/repo/pull/123", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "owner", Repo: "repo", @@ -29,8 +31,8 @@ func TestParseURL(t *testing.T) { { name: "github URL without scheme", input: "github.com/owner/repo/pull/456", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "owner", Repo: "repo", @@ -40,8 +42,8 @@ func TestParseURL(t *testing.T) { { name: "github URL with http", input: "http://github.com/kubernetes/kubernetes/pull/99999", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "kubernetes", Repo: "kubernetes", @@ -51,8 +53,8 @@ func TestParseURL(t *testing.T) { { name: "github URL with dashes in names", input: "https://github.com/my-org/my-repo/pull/1", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "my-org", Repo: "my-repo", @@ -64,8 +66,8 @@ func TestParseURL(t *testing.T) { { name: "gitlab.com MR URL", input: "https://gitlab.com/owner/repo/-/merge_requests/123", - want: &ParsedURL{ - Platform: PlatformGitLab, + want: &types.ParsedURL{ + Platform: types.PlatformGitLab, Host: "gitlab.com", Owner: "owner", Repo: "repo", @@ -75,8 +77,8 @@ func TestParseURL(t *testing.T) { { name: "self-hosted gitlab MR URL", input: "https://gitlab.example.com/team/project/-/merge_requests/456", - want: &ParsedURL{ - Platform: PlatformGitLab, + want: &types.ParsedURL{ + Platform: types.PlatformGitLab, Host: "gitlab.example.com", Owner: "team", Repo: "project", @@ -88,8 +90,8 @@ func TestParseURL(t *testing.T) { { name: "codeberg PR URL", input: "https://codeberg.org/owner/repo/pulls/123", - want: &ParsedURL{ - Platform: PlatformCodeberg, + want: &types.ParsedURL{ + Platform: types.PlatformCodeberg, Host: "codeberg.org", Owner: "owner", Repo: "repo", @@ -99,8 +101,8 @@ func TestParseURL(t *testing.T) { { name: "codeberg URL without scheme", input: "codeberg.org/forgejo/forgejo/pulls/789", - want: &ParsedURL{ - Platform: PlatformCodeberg, + want: &types.ParsedURL{ + Platform: types.PlatformCodeberg, Host: "codeberg.org", Owner: "forgejo", Repo: "forgejo", @@ -112,7 +114,7 @@ func TestParseURL(t *testing.T) { { name: "generic gitea URL", input: "https://gitea.example.com/owner/repo/pulls/123", - want: &ParsedURL{ + want: &types.ParsedURL{ Platform: "gitea", Host: "gitea.example.com", Owner: "owner", @@ -123,7 +125,7 @@ func TestParseURL(t *testing.T) { { name: "unknown host defaults to gitea", input: "https://code.mycompany.com/team/project/pulls/456", - want: &ParsedURL{ + want: &types.ParsedURL{ Platform: "gitea", Host: "code.mycompany.com", Owner: "team", @@ -136,8 +138,8 @@ func TestParseURL(t *testing.T) { { name: "github URL with fragment", input: "https://github.com/owner/repo/pull/123#issuecomment-456", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "owner", Repo: "repo", @@ -147,8 +149,8 @@ func TestParseURL(t *testing.T) { { name: "github URL with query params", input: "https://github.com/owner/repo/pull/123?foo=bar&baz=qux", - want: &ParsedURL{ - Platform: PlatformGitHub, + want: &types.ParsedURL{ + Platform: types.PlatformGitHub, Host: "github.com", Owner: "owner", Repo: "repo", @@ -158,8 +160,8 @@ func TestParseURL(t *testing.T) { { name: "gitlab URL with fragment and query", input: "https://gitlab.com/owner/repo/-/merge_requests/456?tab=notes#note_789", - want: &ParsedURL{ - Platform: PlatformGitLab, + want: &types.ParsedURL{ + Platform: types.PlatformGitLab, Host: "gitlab.com", Owner: "owner", Repo: "repo", @@ -169,7 +171,7 @@ func TestParseURL(t *testing.T) { { name: "gitea URL with query params", input: "https://gitea.example.com/owner/repo/pulls/100?state=open", - want: &ParsedURL{ + want: &types.ParsedURL{ Platform: "gitea", Host: "gitea.example.com", Owner: "owner", @@ -231,21 +233,21 @@ func TestParseURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ParseURL(tt.input) + got, err := types.ParseURL(tt.input) if tt.wantErr { if err == nil { - t.Errorf("ParseURL() expected error containing %q, got nil", tt.errMatch) + t.Errorf("types.ParseURL() expected error containing %q, got nil", tt.errMatch) return } if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { - t.Errorf("ParseURL() error = %q, want error containing %q", err.Error(), tt.errMatch) + t.Errorf("types.ParseURL() error = %q, want error containing %q", err.Error(), tt.errMatch) } return } if err != nil { - t.Errorf("ParseURL() unexpected error: %v", err) + t.Errorf("types.ParseURL() unexpected error: %v", err) return } @@ -273,22 +275,23 @@ func TestDetectPlatform(t *testing.T) { host string want string }{ - {"github.com", PlatformGitHub}, - {"GITHUB.COM", PlatformGitHub}, - {"api.github.com", PlatformGitHub}, - {"gitlab.com", PlatformGitLab}, - {"gitlab.example.com", PlatformGitLab}, - {"my-gitlab.internal", PlatformGitLab}, - {"codeberg.org", PlatformCodeberg}, + {"github.com", types.PlatformGitHub}, + {"GITHUB.COM", types.PlatformGitHub}, + {"api.github.com", types.PlatformGitHub}, + {"gitlab.com", types.PlatformGitLab}, + {"gitlab.example.com", types.PlatformGitLab}, + {"my-gitlab.internal", types.PlatformGitLab}, + {"codeberg.org", types.PlatformCodeberg}, {"example.com", ""}, {"bitbucket.org", ""}, } for _, tt := range tests { t.Run(tt.host, func(t *testing.T) { - got := DetectPlatform(tt.host) + //nolint:staticcheck // Testing deprecated function + got := types.DetectPlatform(tt.host) if got != tt.want { - t.Errorf("DetectPlatform(%q) = %q, want %q", tt.host, got, tt.want) + t.Errorf("types.DetectPlatform(%q) = %q, want %q", tt.host, got, tt.want) } }) } @@ -299,13 +302,13 @@ func TestDetectPlatformFromHost(t *testing.T) { host string want string }{ - {"github.com", PlatformGitHub}, - {"GITHUB.COM", PlatformGitHub}, - {"api.github.com", PlatformGitHub}, - {"gitlab.com", PlatformGitLab}, - {"gitlab.example.com", PlatformGitLab}, - {"my-gitlab.internal", PlatformGitLab}, - {"codeberg.org", PlatformCodeberg}, + {"github.com", types.PlatformGitHub}, + {"GITHUB.COM", types.PlatformGitHub}, + {"api.github.com", types.PlatformGitHub}, + {"gitlab.com", types.PlatformGitLab}, + {"gitlab.example.com", types.PlatformGitLab}, + {"my-gitlab.internal", types.PlatformGitLab}, + {"codeberg.org", types.PlatformCodeberg}, {"example.com", "gitea"}, // Unknown defaults to gitea {"bitbucket.org", "gitea"}, // Unknown defaults to gitea {"gitea.example.com", "gitea"}, // Unknown defaults to gitea @@ -313,7 +316,7 @@ func TestDetectPlatformFromHost(t *testing.T) { for _, tt := range tests { t.Run(tt.host, func(t *testing.T) { - got := detectPlatformFromHost(tt.host) + got := types.DetectPlatformFromHost(tt.host) if got != tt.want { t.Errorf("detectPlatformFromHost(%q) = %q, want %q", tt.host, got, tt.want) } @@ -323,50 +326,50 @@ func TestDetectPlatformFromHost(t *testing.T) { func TestBuildURLs(t *testing.T) { t.Run("GitHub", func(t *testing.T) { - got := BuildGitHubURL("owner", "repo", 123) + got := types.BuildGitHubURL("owner", "repo", 123) want := "https://github.com/owner/repo/pull/123" if got != want { - t.Errorf("BuildGitHubURL() = %q, want %q", got, want) + t.Errorf("types.BuildGitHubURL() = %q, want %q", got, want) } }) t.Run("GitLab default host", func(t *testing.T) { - got := BuildGitLabURL("", "owner", "repo", 456) + got := types.BuildGitLabURL("", "owner", "repo", 456) want := "https://gitlab.com/owner/repo/-/merge_requests/456" if got != want { - t.Errorf("BuildGitLabURL() = %q, want %q", got, want) + t.Errorf("types.BuildGitLabURL() = %q, want %q", got, want) } }) t.Run("GitLab custom host", func(t *testing.T) { - got := BuildGitLabURL("gitlab.example.com", "team", "project", 789) + got := types.BuildGitLabURL("gitlab.example.com", "team", "project", 789) want := "https://gitlab.example.com/team/project/-/merge_requests/789" if got != want { - t.Errorf("BuildGitLabURL() = %q, want %q", got, want) + t.Errorf("types.BuildGitLabURL() = %q, want %q", got, want) } }) t.Run("Codeberg", func(t *testing.T) { - got := BuildCodebergURL("forgejo", "forgejo", 999) + got := types.BuildCodebergURL("forgejo", "forgejo", 999) want := "https://codeberg.org/forgejo/forgejo/pulls/999" if got != want { - t.Errorf("BuildCodebergURL() = %q, want %q", got, want) + t.Errorf("types.BuildCodebergURL() = %q, want %q", got, want) } }) t.Run("Gitea default host", func(t *testing.T) { - got := BuildGiteaURL("", "owner", "repo", 123) + got := types.BuildGiteaURL("", "owner", "repo", 123) want := "https://codeberg.org/owner/repo/pulls/123" if got != want { - t.Errorf("BuildGiteaURL() = %q, want %q", got, want) + t.Errorf("types.BuildGiteaURL() = %q, want %q", got, want) } }) t.Run("Gitea custom host", func(t *testing.T) { - got := BuildGiteaURL("gitea.example.com", "team", "project", 456) + got := types.BuildGiteaURL("gitea.example.com", "team", "project", 456) want := "https://gitea.example.com/team/project/pulls/456" if got != want { - t.Errorf("BuildGiteaURL() = %q, want %q", got, want) + t.Errorf("types.BuildGiteaURL() = %q, want %q", got, want) } }) } @@ -397,19 +400,19 @@ func TestNormalizeURL(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NormalizeURL(tt.input) + got, err := types.NormalizeURL(tt.input) if tt.wantErr { if err == nil { - t.Error("NormalizeURL() expected error, got nil") + t.Error("types.NormalizeURL() expected error, got nil") } return } if err != nil { - t.Errorf("NormalizeURL() unexpected error: %v", err) + t.Errorf("types.NormalizeURL() unexpected error: %v", err) return } if got != tt.want { - t.Errorf("NormalizeURL() = %q, want %q", got, tt.want) + t.Errorf("types.NormalizeURL() = %q, want %q", got, tt.want) } }) } @@ -429,9 +432,9 @@ func TestIsValidPRURL(t *testing.T) { for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { - got := IsValidPRURL(tt.input) + got := types.IsValidPRURL(tt.input) if got != tt.want { - t.Errorf("IsValidPRURL(%q) = %v, want %v", tt.input, got, tt.want) + t.Errorf("types.IsValidPRURL(%q) = %v, want %v", tt.input, got, tt.want) } }) } @@ -450,19 +453,19 @@ func TestExtractShortRef(t *testing.T) { for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { - got, err := ExtractShortRef(tt.input) + got, err := types.ExtractShortRef(tt.input) if tt.wantErr { if err == nil { - t.Error("ExtractShortRef() expected error, got nil") + t.Error("types.ExtractShortRef() expected error, got nil") } return } if err != nil { - t.Errorf("ExtractShortRef() unexpected error: %v", err) + t.Errorf("types.ExtractShortRef() unexpected error: %v", err) return } if got != tt.want { - t.Errorf("ExtractShortRef() = %q, want %q", got, tt.want) + t.Errorf("types.ExtractShortRef() = %q, want %q", got, tt.want) } }) } @@ -517,15 +520,15 @@ func TestParseShortRef(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ParseShortRef(tt.ref) + got, err := types.ParseShortRef(tt.ref) if tt.wantErr { if err == nil { - t.Error("ParseShortRef() expected error, got nil") + t.Error("types.ParseShortRef() expected error, got nil") } return } if err != nil { - t.Errorf("ParseShortRef() unexpected error: %v", err) + t.Errorf("types.ParseShortRef() unexpected error: %v", err) return } if got.Owner != tt.wantOwner { @@ -557,7 +560,7 @@ func TestParseOwnerRepoPR(t *testing.T) { wantOwner: "owner", wantRepo: "repo", wantNumber: 123, - wantPlatform: PlatformGitHub, + wantPlatform: types.PlatformGitHub, }, { name: "short ref with hash", @@ -581,7 +584,7 @@ func TestParseOwnerRepoPR(t *testing.T) { wantOwner: "team", wantRepo: "project", wantNumber: 999, - wantPlatform: PlatformGitLab, + wantPlatform: types.PlatformGitLab, }, { name: "invalid", @@ -592,15 +595,15 @@ func TestParseOwnerRepoPR(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ParseOwnerRepoPR(tt.input) + got, err := types.ParseOwnerRepoPR(tt.input) if tt.wantErr { if err == nil { - t.Error("ParseOwnerRepoPR() expected error, got nil") + t.Error("types.ParseOwnerRepoPR() expected error, got nil") } return } if err != nil { - t.Errorf("ParseOwnerRepoPR() unexpected error: %v", err) + t.Errorf("types.ParseOwnerRepoPR() unexpected error: %v", err) return } if got.Owner != tt.wantOwner { diff --git a/pkg/prx/utils_test.go b/pkg/prx/utils_test.go index 9df987f..4a169fb 100644 --- a/pkg/prx/utils_test.go +++ b/pkg/prx/utils_test.go @@ -3,12 +3,14 @@ package prx import ( "reflect" "testing" + + "github.com/codeGROOVE-dev/prx/pkg/prx/types" ) func TestCalculateCheckSummaryWithMaps(t *testing.T) { tests := []struct { name string - events []Event + events []types.Event requiredChecks []string expectedSuccess map[string]string expectedFailing map[string]string @@ -20,7 +22,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }{ { name: "mixed statuses with descriptions", - events: []Event{ + events: []types.Event{ { Kind: "check_run", Body: "build", @@ -64,7 +66,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }, { name: "missing required checks marked as pending", - events: []Event{ + events: []types.Event{ { Kind: "check_run", Body: "build", @@ -87,7 +89,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }, { name: "action_required counted as failure", - events: []Event{ + events: []types.Event{ { Kind: "check_run", Body: "deploy", @@ -108,7 +110,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }, { name: "cancelled and skipped statuses", - events: []Event{ + events: []types.Event{ { Kind: "check_run", Body: "optional-check", @@ -137,7 +139,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }, { name: "duplicate check names use latest", - events: []Event{ + events: []types.Event{ { Kind: "check_run", Body: "test", @@ -164,7 +166,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { }, { name: "no events with required checks", - events: []Event{}, + events: []types.Event{}, requiredChecks: []string{"build", "test"}, expectedSuccess: map[string]string{}, expectedFailing: map[string]string{}, @@ -181,7 +183,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - summary := CalculateCheckSummary(tt.events, tt.requiredChecks) + summary := types.CalculateCheckSummary(tt.events, tt.requiredChecks) if !reflect.DeepEqual(summary.Success, tt.expectedSuccess) { t.Errorf("Success mismatch\ngot: %v\nwant: %v", summary.Success, tt.expectedSuccess) @@ -210,7 +212,7 @@ func TestCalculateCheckSummaryWithMaps(t *testing.T) { func TestCheckSummaryInitialization(t *testing.T) { // Test that maps are properly initialized even with no events - summary := CalculateCheckSummary([]Event{}, []string{}) + summary := types.CalculateCheckSummary([]types.Event{}, []string{}) if summary.Success == nil { t.Error("Success map should be initialized, not nil") @@ -248,7 +250,7 @@ func TestCheckSummaryInitialization(t *testing.T) { func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { tests := []struct { name string - events []Event + events []types.Event expectedWithAccess int expectedWithUnknownAccess int expectedWithoutAccess int @@ -256,12 +258,12 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }{ { name: "approval with definite write access", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "owner-user", Outcome: "approved", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, }, expectedWithAccess: 1, @@ -270,13 +272,13 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { expectedChangesRequested: 0, }, { - name: "approval with unknown write access (WriteAccessUnlikely)", - events: []Event{ + name: "approval with unknown write access (types.WriteAccessUnlikely)", + events: []types.Event{ { Kind: "review", Actor: "external-contributor", Outcome: "approved", - WriteAccess: WriteAccessUnlikely, + WriteAccess: types.WriteAccessUnlikely, }, }, expectedWithAccess: 0, @@ -286,12 +288,12 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "approval with likely write access", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "member-user", Outcome: "approved", - WriteAccess: WriteAccessLikely, + WriteAccess: types.WriteAccessLikely, }, }, expectedWithAccess: 0, @@ -301,12 +303,12 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "approval with NA write access", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "unknown-user", Outcome: "approved", - WriteAccess: WriteAccessNA, + WriteAccess: types.WriteAccessNA, }, }, expectedWithAccess: 0, @@ -316,12 +318,12 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "approval with confirmed no write access", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "blocked-user", Outcome: "approved", - WriteAccess: WriteAccessNo, + WriteAccess: types.WriteAccessNo, }, }, expectedWithAccess: 0, @@ -331,30 +333,30 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "mixed approvals with different write access levels", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "owner", Outcome: "approved", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, { Kind: "review", Actor: "contributor", Outcome: "approved", - WriteAccess: WriteAccessUnlikely, + WriteAccess: types.WriteAccessUnlikely, }, { Kind: "review", Actor: "member", Outcome: "approved", - WriteAccess: WriteAccessLikely, + WriteAccess: types.WriteAccessLikely, }, { Kind: "review", Actor: "blocked", Outcome: "approved", - WriteAccess: WriteAccessNo, + WriteAccess: types.WriteAccessNo, }, }, expectedWithAccess: 1, @@ -364,18 +366,18 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "latest review overrides previous (approval then changes_requested)", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "reviewer", Outcome: "approved", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, { Kind: "review", Actor: "reviewer", Outcome: "changes_requested", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, }, expectedWithAccess: 0, @@ -385,12 +387,12 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { }, { name: "commented reviews ignored", - events: []Event{ + events: []types.Event{ { Kind: "review", Actor: "commenter", Outcome: "commented", - WriteAccess: WriteAccessDefinitely, + WriteAccess: types.WriteAccessDefinitely, }, }, expectedWithAccess: 0, @@ -402,7 +404,7 @@ func TestCalculateApprovalSummaryWriteAccessCategories(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - summary := CalculateApprovalSummary(tt.events) + summary := types.CalculateApprovalSummary(tt.events) if summary.ApprovalsWithWriteAccess != tt.expectedWithAccess { t.Errorf("ApprovalsWithWriteAccess: got %d, want %d", @@ -428,7 +430,7 @@ func TestCheckSummaryCancelledNotInFailing(t *testing.T) { // Regression test: cancelled checks should only appear in cancelled map, not in failing map // This was a bug where cancelled checks appeared in both maps // Based on real-world scenario from https://github.com/codeGROOVE-dev/goose/pull/65 - events := []Event{ + events := []types.Event{ { Kind: "check_run", Body: "Kusari Inspector", @@ -458,7 +460,7 @@ func TestCheckSummaryCancelledNotInFailing(t *testing.T) { }, } - summary := CalculateCheckSummary(events, []string{}) + summary := types.CalculateCheckSummary(events, []string{}) // Verify cancelled check is ONLY in cancelled map if _, exists := summary.Cancelled["Test (macos-latest)"]; !exists {