From f473f97e247e776bab1974f182cb860f42e1b8d2 Mon Sep 17 00:00:00 2001 From: Edmund Kohlwey Date: Tue, 23 Jun 2026 20:41:01 -0400 Subject: [PATCH] forge: Add ChecksByChange for per-change checks reporting Adds a display-oriented checks layer on top of upstream's per-check ChangeChecks/ChangeCheckState API: - forge.ChecksReport{Rollup, Runs, URL}: a per-change rollup plus per-run detail and a checks summary URL. - forge.ChecksRollupState (pending/passed/failed/none), with a distinct 'none' for no checks configured/reported, and TextMarshaler/Unmarshaler so it serializes as a string. - forge.CheckRun{Name, State, URL}: per-run detail whose State stays a forge-native string so the API need not enumerate every forge's vocabulary. - Repository.ChecksByChange(ids), a batch method mirroring CommentCountsByChange. Each forge stubs ChecksByChange (one nil per id) so the schema lands standalone; real implementations follow per-forge. --- internal/forge/bitbucket/checks_report.go | 19 ++++ internal/forge/forge.go | 133 ++++++++++++++++++++++ internal/forge/forge_test.go | 49 ++++++++ internal/forge/forgetest/mocks.go | 39 +++++++ internal/forge/github/checks_report.go | 19 ++++ internal/forge/gitlab/checks_report.go | 19 ++++ internal/forge/shamhub/checks_report.go | 19 ++++ 7 files changed, 297 insertions(+) create mode 100644 internal/forge/bitbucket/checks_report.go create mode 100644 internal/forge/github/checks_report.go create mode 100644 internal/forge/gitlab/checks_report.go create mode 100644 internal/forge/shamhub/checks_report.go diff --git a/internal/forge/bitbucket/checks_report.go b/internal/forge/bitbucket/checks_report.go new file mode 100644 index 00000000..3bd94d60 --- /dev/null +++ b/internal/forge/bitbucket/checks_report.go @@ -0,0 +1,19 @@ +package bitbucket + +import ( + "context" + + "go.abhg.dev/gs/internal/forge" +) + +// ChecksByChange reports per-change rolled-up and per-run check state +// for each of the given changes. +// +// TODO: real implementation lands on a follow-up branch. +// This stub returns one nil per id to satisfy the [forge.Repository] +// interface while the schema branch lands standalone. +func (r *Repository) ChecksByChange( + _ context.Context, ids []forge.ChangeID, +) ([]*forge.ChecksReport, error) { + return make([]*forge.ChecksReport, len(ids)), nil +} diff --git a/internal/forge/forge.go b/internal/forge/forge.go index 3c855cc5..c7d19ae1 100644 --- a/internal/forge/forge.go +++ b/internal/forge/forge.go @@ -308,6 +308,16 @@ type Repository interface { // or the change has no required checks, // implementations should return an empty slice. ChangeChecks(ctx context.Context, id ChangeID) ([]ChangeCheck, error) + + // ChecksByChange reports per-change rolled-up and per-run check + // state for each of the given changes, in the same order as ids. + // + // A change with no checks configured or reported should yield a + // [*ChecksReport] whose Rollup is [ChecksRollupNone] and whose + // Runs is empty. Implementations may return nil for that slot if + // no data is available at all. + ChecksByChange(ctx context.Context, ids []ChangeID) ([]*ChecksReport, error) + CommentCountsByChange(ctx context.Context, ids []ChangeID) ([]*CommentCounts, error) // Post, update, and delete comments on changes. @@ -938,3 +948,126 @@ func (s ChangeCheckState) GoString() string { return fmt.Sprintf("ChangeCheckState(%d)", int(s)) } } + +// ChecksReport describes the rolled-up and per-run check status for +// a change. +// +// It is a display-oriented view layered on top of the per-check +// [ChangeCheck] primitive: [Repository.ChecksByChange] reports one +// per change for surfaces that want a single rollup plus per-run +// detail (e.g. gs log --cr-checks). +type ChecksReport struct { + // Rollup is the single rolled-up state across all reported + // checks. Drives UI indicators that need a single visual. + Rollup ChecksRollupState + + // Runs lists the individual checks/runs the forge reported, in + // forge-defined order. Empty when Rollup is [ChecksRollupNone]. + Runs []CheckRun + + // URL is the forge's summary page for the change's checks. + // May be empty if the forge has no per-change checks page or if + // no checks are reported. + URL string +} + +// CheckRun is a single check reported by the forge. +// +// State is left as a forge-native string (e.g. GitHub's "success", +// "neutral", "timed_out") so consumers can distinguish nuances the +// rollup collapses. +type CheckRun struct { + // Name is the forge-defined display name of the check. + Name string + + // State is the forge-native state string for this run. + State string + + // URL is the forge's detail page for this run, if any. + URL string +} + +// ChecksRollupState represents the rolled-up CI/checks status for a +// change. +// +// The enum collapses each forge's native taxonomy down to the four +// states that drive UI distinction (pending, passed, failed, none). +// Forge-native fidelity is preserved per-run in [CheckRun.State]. +type ChecksRollupState int + +const ( + // ChecksRollupPending indicates checks are still running. + ChecksRollupPending ChecksRollupState = iota + 1 + + // ChecksRollupPassed indicates all checks have passed. + ChecksRollupPassed + + // ChecksRollupFailed indicates one or more checks failed. + ChecksRollupFailed + + // ChecksRollupNone indicates that no checks are configured or + // reported for the change. Distinct from ChecksRollupPassed: + // the absence of checks is a different UI state than a green + // outcome. + ChecksRollupNone +) + +func (s ChecksRollupState) String() string { + switch s { + case ChecksRollupPending: + return "pending" + case ChecksRollupPassed: + return "passed" + case ChecksRollupFailed: + return "failed" + case ChecksRollupNone: + return "none" + default: + return fmt.Sprintf("ChecksRollupState(%d)", int(s)) + } +} + +// GoString returns a Go-syntax representation. +func (s ChecksRollupState) GoString() string { + switch s { + case ChecksRollupPending: + return "ChecksRollupPending" + case ChecksRollupPassed: + return "ChecksRollupPassed" + case ChecksRollupFailed: + return "ChecksRollupFailed" + case ChecksRollupNone: + return "ChecksRollupNone" + default: + return fmt.Sprintf("ChecksRollupState(%d)", int(s)) + } +} + +// MarshalText encodes the state as one of "pending", "passed", +// "failed", or "none". This implements [encoding.TextMarshaler]. +func (s ChecksRollupState) MarshalText() ([]byte, error) { + switch s { + case ChecksRollupPending, ChecksRollupPassed, ChecksRollupFailed, ChecksRollupNone: + return []byte(s.String()), nil + default: + return nil, fmt.Errorf("unknown checks rollup state: %d", int(s)) + } +} + +// UnmarshalText decodes a state from one of "pending", "passed", +// "failed", or "none". This implements [encoding.TextUnmarshaler]. +func (s *ChecksRollupState) UnmarshalText(b []byte) error { + switch string(b) { + case "pending": + *s = ChecksRollupPending + case "passed": + *s = ChecksRollupPassed + case "failed": + *s = ChecksRollupFailed + case "none": + *s = ChecksRollupNone + default: + return fmt.Errorf("unknown checks rollup state: %q", b) + } + return nil +} diff --git a/internal/forge/forge_test.go b/internal/forge/forge_test.go index 53288400..792fec9d 100644 --- a/internal/forge/forge_test.go +++ b/internal/forge/forge_test.go @@ -310,6 +310,55 @@ func TestChangeState(t *testing.T) { }) } +func TestChecksRollupState(t *testing.T) { + tests := []struct { + state forge.ChecksRollupState + str string + }{ + {forge.ChecksRollupPending, "pending"}, + {forge.ChecksRollupPassed, "passed"}, + {forge.ChecksRollupFailed, "failed"}, + {forge.ChecksRollupNone, "none"}, + } + + for _, tt := range tests { + t.Run(tt.str, func(t *testing.T) { + t.Run("String", func(t *testing.T) { + assert.Equal(t, tt.str, tt.state.String()) + }) + + t.Run("MarshalRoundTrip", func(t *testing.T) { + bs, err := tt.state.MarshalText() + require.NoError(t, err) + assert.Equal(t, tt.str, string(bs)) + + var s forge.ChecksRollupState + require.NoError(t, s.UnmarshalText(bs)) + assert.Equal(t, tt.state, s) + }) + }) + } + + t.Run("unknown", func(t *testing.T) { + s := forge.ChecksRollupState(42) + + t.Run("String", func(t *testing.T) { + assert.Equal(t, "ChecksRollupState(42)", s.String()) + }) + + t.Run("Marshal", func(t *testing.T) { + _, err := s.MarshalText() + assert.Error(t, err) + }) + + t.Run("Unmarshal", func(t *testing.T) { + var got forge.ChecksRollupState + err := got.UnmarshalText([]byte("unknown")) + assert.Error(t, err) + }) + }) +} + func TestMergeMethod(t *testing.T) { tests := []struct { method forge.MergeMethod diff --git a/internal/forge/forgetest/mocks.go b/internal/forge/forgetest/mocks.go index a146405a..d8756d28 100644 --- a/internal/forge/forgetest/mocks.go +++ b/internal/forge/forgetest/mocks.go @@ -826,6 +826,45 @@ func (c *MockRepositoryChangeStatusesCall) DoAndReturn(f func(context.Context, [ return c } +// ChecksByChange mocks base method. +func (m *MockRepository) ChecksByChange(ctx context.Context, ids []forge.ChangeID) ([]*forge.ChecksReport, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ChecksByChange", ctx, ids) + ret0, _ := ret[0].([]*forge.ChecksReport) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ChecksByChange indicates an expected call of ChecksByChange. +func (mr *MockRepositoryMockRecorder) ChecksByChange(ctx, ids any) *MockRepositoryChecksByChangeCall { + mr.mock.ctrl.T.Helper() + call := mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ChecksByChange", reflect.TypeOf((*MockRepository)(nil).ChecksByChange), ctx, ids) + return &MockRepositoryChecksByChangeCall{Call: call} +} + +// MockRepositoryChecksByChangeCall wrap *gomock.Call +type MockRepositoryChecksByChangeCall struct { + *gomock.Call +} + +// Return rewrite *gomock.Call.Return +func (c *MockRepositoryChecksByChangeCall) Return(arg0 []*forge.ChecksReport, arg1 error) *MockRepositoryChecksByChangeCall { + c.Call = c.Call.Return(arg0, arg1) + return c +} + +// Do rewrite *gomock.Call.Do +func (c *MockRepositoryChecksByChangeCall) Do(f func(context.Context, []forge.ChangeID) ([]*forge.ChecksReport, error)) *MockRepositoryChecksByChangeCall { + c.Call = c.Call.Do(f) + return c +} + +// DoAndReturn rewrite *gomock.Call.DoAndReturn +func (c *MockRepositoryChecksByChangeCall) DoAndReturn(f func(context.Context, []forge.ChangeID) ([]*forge.ChecksReport, error)) *MockRepositoryChecksByChangeCall { + c.Call = c.Call.DoAndReturn(f) + return c +} + // CommentCountsByChange mocks base method. func (m *MockRepository) CommentCountsByChange(ctx context.Context, ids []forge.ChangeID) ([]*forge.CommentCounts, error) { m.ctrl.T.Helper() diff --git a/internal/forge/github/checks_report.go b/internal/forge/github/checks_report.go new file mode 100644 index 00000000..be06dca6 --- /dev/null +++ b/internal/forge/github/checks_report.go @@ -0,0 +1,19 @@ +package github + +import ( + "context" + + "go.abhg.dev/gs/internal/forge" +) + +// ChecksByChange reports per-change rolled-up and per-run check state +// for each of the given changes. +// +// TODO: real implementation lands on a follow-up branch. +// This stub returns one nil per id to satisfy the [forge.Repository] +// interface while the schema branch lands standalone. +func (r *Repository) ChecksByChange( + _ context.Context, ids []forge.ChangeID, +) ([]*forge.ChecksReport, error) { + return make([]*forge.ChecksReport, len(ids)), nil +} diff --git a/internal/forge/gitlab/checks_report.go b/internal/forge/gitlab/checks_report.go new file mode 100644 index 00000000..71598189 --- /dev/null +++ b/internal/forge/gitlab/checks_report.go @@ -0,0 +1,19 @@ +package gitlab + +import ( + "context" + + "go.abhg.dev/gs/internal/forge" +) + +// ChecksByChange reports per-change rolled-up and per-run check state +// for each of the given changes. +// +// TODO: real implementation lands on a follow-up branch. +// This stub returns one nil per id to satisfy the [forge.Repository] +// interface while the schema branch lands standalone. +func (r *Repository) ChecksByChange( + _ context.Context, ids []forge.ChangeID, +) ([]*forge.ChecksReport, error) { + return make([]*forge.ChecksReport, len(ids)), nil +} diff --git a/internal/forge/shamhub/checks_report.go b/internal/forge/shamhub/checks_report.go new file mode 100644 index 00000000..aa2bc999 --- /dev/null +++ b/internal/forge/shamhub/checks_report.go @@ -0,0 +1,19 @@ +package shamhub + +import ( + "context" + + "go.abhg.dev/gs/internal/forge" +) + +// ChecksByChange reports per-change rolled-up and per-run check state +// for each of the given changes. +// +// TODO: real implementation lands on a follow-up branch. +// This stub returns one nil per id to satisfy the [forge.Repository] +// interface while the schema branch lands standalone. +func (r *forgeRepository) ChecksByChange( + _ context.Context, ids []forge.ChangeID, +) ([]*forge.ChecksReport, error) { + return make([]*forge.ChecksReport, len(ids)), nil +}