From fb1d976bf70a7cd16b018a939f50c38bd4403155 Mon Sep 17 00:00:00 2001 From: "chenghan.ying" Date: Tue, 14 Jul 2026 21:30:57 +0000 Subject: [PATCH 1/3] feat(stovepipe): add buildrunner extension contract and fake --- stovepipe/extension/buildrunner/BUILD.bazel | 9 ++ stovepipe/extension/buildrunner/README.md | 21 +++ .../extension/buildrunner/buildrunner.go | 86 +++++++++++ .../extension/buildrunner/fake/BUILD.bazel | 24 +++ stovepipe/extension/buildrunner/fake/fake.go | 137 ++++++++++++++++++ .../extension/buildrunner/fake/fake_test.go | 112 ++++++++++++++ 6 files changed, 389 insertions(+) create mode 100644 stovepipe/extension/buildrunner/BUILD.bazel create mode 100644 stovepipe/extension/buildrunner/README.md create mode 100644 stovepipe/extension/buildrunner/buildrunner.go create mode 100644 stovepipe/extension/buildrunner/fake/BUILD.bazel create mode 100644 stovepipe/extension/buildrunner/fake/fake.go create mode 100644 stovepipe/extension/buildrunner/fake/fake_test.go diff --git a/stovepipe/extension/buildrunner/BUILD.bazel b/stovepipe/extension/buildrunner/BUILD.bazel new file mode 100644 index 00000000..0f5451e6 --- /dev/null +++ b/stovepipe/extension/buildrunner/BUILD.bazel @@ -0,0 +1,9 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["buildrunner.go"], + importpath = "github.com/uber/submitqueue/stovepipe/extension/buildrunner", + visibility = ["//visibility:public"], + deps = ["//stovepipe/entity:go_default_library"], +) diff --git a/stovepipe/extension/buildrunner/README.md b/stovepipe/extension/buildrunner/README.md new file mode 100644 index 00000000..2268604f --- /dev/null +++ b/stovepipe/extension/buildrunner/README.md @@ -0,0 +1,21 @@ +# BuildRunner + +Vendor-agnostic interface through which Stovepipe triggers and polls builds against an external build system. Shaped the same as SubmitQueue's own `buildrunner` extension — same `Trigger`/`Status`/`Cancel` verbs, same async contract, same id model — but a separate interface rather than a shared one: Stovepipe validates a single commit against a baseline (or from scratch), not a stack of dependency batches, so `Trigger` takes URI identity instead of batch identity. See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for the full rationale, and its ["Carries over vs. new"](../../../doc/rfc/stovepipe/steps/build.md#carries-over-vs-new) section for what is shaped the same as SubmitQueue's versus what is Stovepipe-specific. + +Per the repository's extension rules, this package holds the `BuildRunner` interface, its `Config`, and the `Factory` *interface* only — concrete `Factory` implementations and the per-queue routing that picks a backend for a `Config.QueueName` live in the wiring layer. + +## Behavior + +- **Trigger** starts a new build against `headURI` (optionally relative to an incremental `baseURI`) and returns the runner-minted build id. There is no caller-supplied dedup input — every call starts a fresh build, and downstream idempotency absorbs any duplicate from a redelivery. Trigger must return promptly; the build itself runs asynchronously. +- **Status** polls the current status and any provider metadata for a build id `Trigger` returned. Unlike `Trigger`, it may round-trip to the backend and block. +- **Cancel** requests cancellation for a build id, returning once the request reaches the runner rather than once the build actually stops. No controller calls it today — it exists for contract parity with SubmitQueue and for future use. + +## Errors + +Implementations return plain, unclassified errors — the calling controller decides user-vs-infra and retryable-vs-not, per `platform/errs`. There is no package error sentinel yet; a domain sentinel (e.g. for "unknown build") is deferred until a concrete need for it lands. + +## Implementations + +- **fake** — a stateless backend that succeeds by default and honors failure-injection markers embedded in `headURI`, for examples and tests. + +To add a backend, create `buildrunner/{backend}/`, implement the `BuildRunner` interface, and return it from a `New(...)` constructor. diff --git a/stovepipe/extension/buildrunner/buildrunner.go b/stovepipe/extension/buildrunner/buildrunner.go new file mode 100644 index 00000000..b9f73d65 --- /dev/null +++ b/stovepipe/extension/buildrunner/buildrunner.go @@ -0,0 +1,86 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package buildrunner defines the contract through which Stovepipe triggers and +// polls builds against an external build system. It is shaped the same as +// SubmitQueue's own buildrunner extension — same Trigger/Status/Cancel verbs, +// same async contract, same id model — but is a separate interface rather than a +// shared one: Stovepipe validates a single commit against a baseline (or from +// scratch), not a stack of dependency batches, so Trigger takes URI identity +// instead of batch identity. See doc/rfc/stovepipe/steps/build.md's "Why separate +// contracts" for the full rationale. +package buildrunner + +//go:generate mockgen -source=buildrunner.go -destination=mock/buildrunner_mock.go -package=mock + +import ( + "context" + + "github.com/uber/submitqueue/stovepipe/entity" +) + +// BuildRunner triggers builds against an external build system, polls their +// status, and cancels them. Implementations are long-lived singletons and must: +// - be safe for concurrent use by multiple goroutines; +// - recover from transient connectivity failures internally, returning plain +// errors during the recovery window rather than blocking the caller +// indefinitely; +// - keep only transient local state (caches, pools) — the durable link between +// a Request and its Build lives in BuildStore, never in the runner; +// - return plain, unclassified errors and leave user-vs-infra and +// retryable-vs-not classification to the calling controller, per +// platform/errs. +type BuildRunner interface { + // Trigger starts a new build every call and mints the build's identity — + // there is no caller-supplied dedup input. headURI is the commit under + // validation; baseURI is the incremental baseline, empty for a full + // build; both are opaque tokens owned by SourceControl. metadata is + // caller-supplied annotation the runner may echo back via Status but must + // not depend on. Trigger is async: it must return promptly with the + // runner-assigned id, not an outcome — callers learn progress via Status. + Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) + + // Status returns the build's current status and any provider metadata for + // the id Trigger returned. Unlike Trigger, Status may round-trip to the + // backend and block. The returned BuildMetadata is caller-supplied, + // provider-echoed — the runner must not depend on it, but a caller may + // read it for its own purposes. + Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) + + // Cancel requests cancellation of the build for the id Trigger returned, + // returning once the request reaches the runner, not once the build + // actually stops. A no-op on an already-terminal build. No controller + // calls Cancel today — see doc/rfc/stovepipe/steps/build.md's + // "Cancellation: defined, not yet called" — it exists for contract parity + // and future use. + Cancel(ctx context.Context, buildID entity.BuildID) error +} + +// Config carries the per-queue identity handed to a Factory. It is the only +// identity the system hands a Factory; everything else a concrete BuildRunner +// needs (endpoint, credentials, pipeline mapping) is injected by the integrator +// at construction. +type Config struct { + // QueueName identifies which Queue's build-runner backend to resolve. + QueueName string +} + +// Factory resolves the BuildRunner for a Config. Implementations and the +// per-queue routing that picks a backend for a Config.QueueName live in the +// wiring layer (service/stovepipe/.../server/main.go), not here — see +// CLAUDE.md's extension rules. +type Factory interface { + // For returns the BuildRunner for the given queue. + For(cfg Config) (BuildRunner, error) +} diff --git a/stovepipe/extension/buildrunner/fake/BUILD.bazel b/stovepipe/extension/buildrunner/fake/BUILD.bazel new file mode 100644 index 00000000..3157397b --- /dev/null +++ b/stovepipe/extension/buildrunner/fake/BUILD.bazel @@ -0,0 +1,24 @@ +load("@rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["fake.go"], + importpath = "github.com/uber/submitqueue/stovepipe/extension/buildrunner/fake", + visibility = ["//visibility:public"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/buildrunner:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["fake_test.go"], + embed = [":go_default_library"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/buildrunner:go_default_library", + "@com_github_stretchr_testify//assert:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + ], +) diff --git a/stovepipe/extension/buildrunner/fake/fake.go b/stovepipe/extension/buildrunner/fake/fake.go new file mode 100644 index 00000000..b4b82b31 --- /dev/null +++ b/stovepipe/extension/buildrunner/fake/fake.go @@ -0,0 +1,137 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package fake provides a buildrunner.BuildRunner whose outcome is driven by the +// triggered head URI. With no marker every build immediately succeeds, behaving +// as a best-case stub for local-stack/e2e wiring. Failures are injected by +// embedding a marker token in headURI of the form "buildrunner-fake=": +// +// buildrunner-fake=trigger-error -> Trigger returns a non-nil error +// buildrunner-fake=build-fail -> Status reports BuildStatusFailed +// buildrunner-fake=build-error -> Status returns a non-nil error +// +// The runner is stateless: Trigger encodes the desired terminal outcome into the +// returned BuildID, and Status decides the result purely from the BuildID it is +// given — no per-build bookkeeping. This means any runner instance can answer +// Status for an id minted by any other (Trigger and Status can even live in +// different processes), and a single running stack can exercise the negative +// paths purely by varying request payloads. It is intended for examples and +// tests only, never production. +package fake + +import ( + "context" + "crypto/rand" + "encoding/hex" + "fmt" + "strings" + + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/buildrunner" +) + +// markerPrefix introduces a marker token in headURI: "buildrunner-fake=". +const markerPrefix = "buildrunner-fake=" + +// Recognized marker tokens. See the package doc for the convention. +const ( + tokenTriggerError = "trigger-error" + tokenFail = "build-fail" + tokenError = "build-error" +) + +// outcomeOK is the BuildID outcome segment for a build that should succeed. +const outcomeOK = "ok" + +// runner is a buildrunner.BuildRunner that reports every build as succeeded +// unless a marker token in headURI requests otherwise. It holds no per-build +// state: the outcome is encoded in the BuildID at Trigger and read back out at +// Status. Uniqueness comes from a random suffix per id, so it needs no shared +// counter and never collides across instances or processes. +type runner struct{} + +// New returns a buildrunner.BuildRunner that defaults to succeeding and honors +// marker tokens embedded in the triggered headURI. +func New() buildrunner.BuildRunner { + return runner{} +} + +// Trigger fails when headURI carries the trigger-error marker; otherwise it +// returns a unique BuildID that encodes the terminal outcome the build should +// report at Status time (decided from the headURI marker). baseURI and metadata +// are ignored. +func (r runner) Trigger(_ context.Context, headURI, _ string, _ entity.BuildMetadata) (entity.BuildID, error) { + outcome := outcomeOK + switch marker(headURI) { + case tokenTriggerError: + return entity.BuildID{}, fmt.Errorf("fake: marked trigger error") + case tokenFail: + outcome = tokenFail + case tokenError: + outcome = tokenError + } + + // Encode the outcome in the id (e.g. "fake-build-fail-a1b2c3d4") so Status is + // stateless. The random suffix keeps ids globally unique across instances and + // processes without any shared state. + suffix, err := randomSuffix() + if err != nil { + return entity.BuildID{}, fmt.Errorf("fake: generating build id: %w", err) + } + return entity.BuildID{ID: fmt.Sprintf("fake-%s-%s", outcome, suffix)}, nil +} + +// Status decides the result purely from the BuildID's encoded outcome. Ids that +// carry no recognized outcome (including those not minted by this fake) default +// to succeeded, keeping the runner best-case. +func (r runner) Status(_ context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { + switch { + case strings.Contains(buildID.ID, tokenError): + return entity.BuildStatusUnknown, nil, fmt.Errorf("fake: marked build error") + case strings.Contains(buildID.ID, tokenFail): + return entity.BuildStatusFailed, nil, nil + default: + return entity.BuildStatusSucceeded, nil, nil + } +} + +// Cancel is a no-op and always succeeds. +func (r runner) Cancel(_ context.Context, _ entity.BuildID) error { + return nil +} + +// marker returns the marker token embedded in uri, or "" if none is present. +// The token ends at the first "&" or "#" delimiter, so a marker may sit among +// other query parameters or a fragment. +func marker(uri string) string { + _, rest, found := strings.Cut(uri, markerPrefix) + if !found { + return "" + } + if i := strings.IndexAny(rest, "&#"); i >= 0 { + rest = rest[:i] + } + return rest +} + +// randomSuffix returns a short random hex string used to keep fake BuildIDs +// globally unique. Hex digits never spell the outcome marker tokens, so the +// suffix cannot interfere with Status decoding the outcome via substring match. +func randomSuffix() (string, error) { + var b [4]byte + if _, err := rand.Read(b[:]); err != nil { + return "", err + } + return hex.EncodeToString(b[:]), nil +} diff --git a/stovepipe/extension/buildrunner/fake/fake_test.go b/stovepipe/extension/buildrunner/fake/fake_test.go new file mode 100644 index 00000000..f4252bc4 --- /dev/null +++ b/stovepipe/extension/buildrunner/fake/fake_test.go @@ -0,0 +1,112 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fake + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/buildrunner" +) + +func TestNew_ImplementsInterface(t *testing.T) { + var _ buildrunner.BuildRunner = New() +} + +func TestTrigger(t *testing.T) { + tests := []struct { + name string + headURI string + wantErr bool + }{ + {name: "no marker succeeds", headURI: "git://repo/ref/deadbeef"}, + {name: "unrelated query params succeed", headURI: "git://repo/ref/deadbeef?attempt=2"}, + {name: "trigger-error marker fails", headURI: "git://repo/ref/deadbeef?buildrunner-fake=trigger-error", wantErr: true}, + {name: "build-fail marker still triggers", headURI: "git://repo/ref/deadbeef?buildrunner-fake=build-fail"}, + {name: "build-error marker still triggers", headURI: "git://repo/ref/deadbeef?buildrunner-fake=build-error"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + id, err := New().Trigger(context.Background(), tt.headURI, "", nil) + if tt.wantErr { + require.Error(t, err) + assert.Empty(t, id.ID) + return + } + require.NoError(t, err) + assert.NotEmpty(t, id.ID) + }) + } +} + +func TestTrigger_UniqueIDs(t *testing.T) { + a, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef", "", nil) + require.NoError(t, err) + b, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef", "", nil) + require.NoError(t, err) + assert.NotEqual(t, a.ID, b.ID) +} + +func TestStatus(t *testing.T) { + tests := []struct { + name string + headURI string + wantStatus entity.BuildStatus + wantErr bool + }{ + {name: "no marker succeeds", headURI: "git://repo/ref/deadbeef", wantStatus: entity.BuildStatusSucceeded}, + {name: "build-fail marker fails", headURI: "git://repo/ref/deadbeef?buildrunner-fake=build-fail", wantStatus: entity.BuildStatusFailed}, + {name: "build-error marker errors", headURI: "git://repo/ref/deadbeef?buildrunner-fake=build-error", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + id, err := New().Trigger(context.Background(), tt.headURI, "", nil) + require.NoError(t, err) + + status, metadata, err := New().Status(context.Background(), id) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantStatus, status) + assert.Nil(t, metadata) + }) + } +} + +func TestStatus_UnrecognizedIDSucceeds(t *testing.T) { + status, metadata, err := New().Status(context.Background(), entity.BuildID{ID: "not-minted-by-this-fake"}) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusSucceeded, status) + assert.Nil(t, metadata) +} + +func TestStatus_StatelessAcrossInstances(t *testing.T) { + id, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef?buildrunner-fake=build-fail", "", nil) + require.NoError(t, err) + + status, _, err := New().Status(context.Background(), id) + require.NoError(t, err) + assert.Equal(t, entity.BuildStatusFailed, status) +} + +func TestCancel_NoOp(t *testing.T) { + err := New().Cancel(context.Background(), entity.BuildID{ID: "anything"}) + assert.NoError(t, err) +} From 4e0be98d199e40b8d964e7d6e894f9664b5e639b Mon Sep 17 00:00:00 2001 From: "chenghan.ying" Date: Tue, 14 Jul 2026 21:31:02 +0000 Subject: [PATCH 2/3] chore(stovepipe): add buildrunner mocks --- .../extension/buildrunner/mock/BUILD.bazel | 13 ++ .../buildrunner/mock/buildrunner_mock.go | 127 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 stovepipe/extension/buildrunner/mock/BUILD.bazel create mode 100644 stovepipe/extension/buildrunner/mock/buildrunner_mock.go diff --git a/stovepipe/extension/buildrunner/mock/BUILD.bazel b/stovepipe/extension/buildrunner/mock/BUILD.bazel new file mode 100644 index 00000000..5e4d317b --- /dev/null +++ b/stovepipe/extension/buildrunner/mock/BUILD.bazel @@ -0,0 +1,13 @@ +load("@rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["buildrunner_mock.go"], + importpath = "github.com/uber/submitqueue/stovepipe/extension/buildrunner/mock", + visibility = ["//visibility:public"], + deps = [ + "//stovepipe/entity:go_default_library", + "//stovepipe/extension/buildrunner:go_default_library", + "@org_uber_go_mock//gomock:go_default_library", + ], +) diff --git a/stovepipe/extension/buildrunner/mock/buildrunner_mock.go b/stovepipe/extension/buildrunner/mock/buildrunner_mock.go new file mode 100644 index 00000000..5b943474 --- /dev/null +++ b/stovepipe/extension/buildrunner/mock/buildrunner_mock.go @@ -0,0 +1,127 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: buildrunner.go +// +// Generated by this command: +// +// mockgen -source=buildrunner.go -destination=mock/buildrunner_mock.go -package=mock +// + +// Package mock is a generated GoMock package. +package mock + +import ( + context "context" + reflect "reflect" + + entity "github.com/uber/submitqueue/stovepipe/entity" + buildrunner "github.com/uber/submitqueue/stovepipe/extension/buildrunner" + gomock "go.uber.org/mock/gomock" +) + +// MockBuildRunner is a mock of BuildRunner interface. +type MockBuildRunner struct { + ctrl *gomock.Controller + recorder *MockBuildRunnerMockRecorder + isgomock struct{} +} + +// MockBuildRunnerMockRecorder is the mock recorder for MockBuildRunner. +type MockBuildRunnerMockRecorder struct { + mock *MockBuildRunner +} + +// NewMockBuildRunner creates a new mock instance. +func NewMockBuildRunner(ctrl *gomock.Controller) *MockBuildRunner { + mock := &MockBuildRunner{ctrl: ctrl} + mock.recorder = &MockBuildRunnerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockBuildRunner) EXPECT() *MockBuildRunnerMockRecorder { + return m.recorder +} + +// Cancel mocks base method. +func (m *MockBuildRunner) Cancel(ctx context.Context, buildID entity.BuildID) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Cancel", ctx, buildID) + ret0, _ := ret[0].(error) + return ret0 +} + +// Cancel indicates an expected call of Cancel. +func (mr *MockBuildRunnerMockRecorder) Cancel(ctx, buildID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Cancel", reflect.TypeOf((*MockBuildRunner)(nil).Cancel), ctx, buildID) +} + +// Status mocks base method. +func (m *MockBuildRunner) Status(ctx context.Context, buildID entity.BuildID) (entity.BuildStatus, entity.BuildMetadata, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Status", ctx, buildID) + ret0, _ := ret[0].(entity.BuildStatus) + ret1, _ := ret[1].(entity.BuildMetadata) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// Status indicates an expected call of Status. +func (mr *MockBuildRunnerMockRecorder) Status(ctx, buildID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Status", reflect.TypeOf((*MockBuildRunner)(nil).Status), ctx, buildID) +} + +// Trigger mocks base method. +func (m *MockBuildRunner) Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Trigger", ctx, headURI, baseURI, metadata) + ret0, _ := ret[0].(entity.BuildID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Trigger indicates an expected call of Trigger. +func (mr *MockBuildRunnerMockRecorder) Trigger(ctx, headURI, baseURI, metadata any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Trigger", reflect.TypeOf((*MockBuildRunner)(nil).Trigger), ctx, headURI, baseURI, metadata) +} + +// MockFactory is a mock of Factory interface. +type MockFactory struct { + ctrl *gomock.Controller + recorder *MockFactoryMockRecorder + isgomock struct{} +} + +// MockFactoryMockRecorder is the mock recorder for MockFactory. +type MockFactoryMockRecorder struct { + mock *MockFactory +} + +// NewMockFactory creates a new mock instance. +func NewMockFactory(ctrl *gomock.Controller) *MockFactory { + mock := &MockFactory{ctrl: ctrl} + mock.recorder = &MockFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockFactory) EXPECT() *MockFactoryMockRecorder { + return m.recorder +} + +// For mocks base method. +func (m *MockFactory) For(cfg buildrunner.Config) (buildrunner.BuildRunner, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "For", cfg) + ret0, _ := ret[0].(buildrunner.BuildRunner) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// For indicates an expected call of For. +func (mr *MockFactoryMockRecorder) For(cfg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "For", reflect.TypeOf((*MockFactory)(nil).For), cfg) +} From c50b42dea52da68cbdb198c90710a738963f7ef9 Mon Sep 17 00:00:00 2001 From: "chenghan.ying" Date: Mon, 20 Jul 2026 22:31:16 +0000 Subject: [PATCH 3/3] fix(stovepipe): reverse BuildRunner.Trigger arg order, trim README --- stovepipe/extension/buildrunner/README.md | 20 +++++-------------- .../extension/buildrunner/buildrunner.go | 8 ++++---- stovepipe/extension/buildrunner/fake/fake.go | 2 +- .../extension/buildrunner/fake/fake_test.go | 10 +++++----- .../buildrunner/mock/buildrunner_mock.go | 8 ++++---- 5 files changed, 19 insertions(+), 29 deletions(-) diff --git a/stovepipe/extension/buildrunner/README.md b/stovepipe/extension/buildrunner/README.md index 2268604f..3abb05c0 100644 --- a/stovepipe/extension/buildrunner/README.md +++ b/stovepipe/extension/buildrunner/README.md @@ -1,21 +1,11 @@ # BuildRunner -Vendor-agnostic interface through which Stovepipe triggers and polls builds against an external build system. Shaped the same as SubmitQueue's own `buildrunner` extension — same `Trigger`/`Status`/`Cancel` verbs, same async contract, same id model — but a separate interface rather than a shared one: Stovepipe validates a single commit against a baseline (or from scratch), not a stack of dependency batches, so `Trigger` takes URI identity instead of batch identity. See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for the full rationale, and its ["Carries over vs. new"](../../../doc/rfc/stovepipe/steps/build.md#carries-over-vs-new) section for what is shaped the same as SubmitQueue's versus what is Stovepipe-specific. +Vendor-agnostic interface through which Stovepipe triggers and polls builds against an external build system. -Per the repository's extension rules, this package holds the `BuildRunner` interface, its `Config`, and the `Factory` *interface* only — concrete `Factory` implementations and the per-queue routing that picks a backend for a `Config.QueueName` live in the wiring layer. - -## Behavior - -- **Trigger** starts a new build against `headURI` (optionally relative to an incremental `baseURI`) and returns the runner-minted build id. There is no caller-supplied dedup input — every call starts a fresh build, and downstream idempotency absorbs any duplicate from a redelivery. Trigger must return promptly; the build itself runs asynchronously. +- **Trigger** starts a new build against `headURI`, optionally relative to an incremental `baseURI`, and returns the runner-minted build id. There is no caller-supplied dedup input — every call starts a fresh build; downstream idempotency absorbs any duplicate from a redelivery. Trigger must return promptly; the build itself runs asynchronously. - **Status** polls the current status and any provider metadata for a build id `Trigger` returned. Unlike `Trigger`, it may round-trip to the backend and block. -- **Cancel** requests cancellation for a build id, returning once the request reaches the runner rather than once the build actually stops. No controller calls it today — it exists for contract parity with SubmitQueue and for future use. - -## Errors - -Implementations return plain, unclassified errors — the calling controller decides user-vs-infra and retryable-vs-not, per `platform/errs`. There is no package error sentinel yet; a domain sentinel (e.g. for "unknown build") is deferred until a concrete need for it lands. - -## Implementations +- **Cancel** requests cancellation for a build id, returning once the request reaches the runner rather than once the build actually stops. Unused today; kept for contract parity with SubmitQueue. -- **fake** — a stateless backend that succeeds by default and honors failure-injection markers embedded in `headURI`, for examples and tests. +Implementations return plain, unclassified errors — the calling controller decides retryable-vs-not and user-vs-infra, per `platform/errs`. -To add a backend, create `buildrunner/{backend}/`, implement the `BuildRunner` interface, and return it from a `New(...)` constructor. +See [doc/rfc/stovepipe/steps/build.md](../../../doc/rfc/stovepipe/steps/build.md#why-separate-contracts) for why this is a separate contract from SubmitQueue's own `buildrunner` rather than a shared one. To add a backend, create `buildrunner/{backend}/`, implement `BuildRunner`, and return it from a `New(...)` constructor. diff --git a/stovepipe/extension/buildrunner/buildrunner.go b/stovepipe/extension/buildrunner/buildrunner.go index b9f73d65..4afa39be 100644 --- a/stovepipe/extension/buildrunner/buildrunner.go +++ b/stovepipe/extension/buildrunner/buildrunner.go @@ -43,13 +43,13 @@ import ( // platform/errs. type BuildRunner interface { // Trigger starts a new build every call and mints the build's identity — - // there is no caller-supplied dedup input. headURI is the commit under - // validation; baseURI is the incremental baseline, empty for a full - // build; both are opaque tokens owned by SourceControl. metadata is + // there is no caller-supplied dedup input. baseURI is the incremental + // baseline, empty for a full build; headURI is the commit under + // validation; both are opaque tokens owned by SourceControl. metadata is // caller-supplied annotation the runner may echo back via Status but must // not depend on. Trigger is async: it must return promptly with the // runner-assigned id, not an outcome — callers learn progress via Status. - Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) + Trigger(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error) // Status returns the build's current status and any provider metadata for // the id Trigger returned. Unlike Trigger, Status may round-trip to the diff --git a/stovepipe/extension/buildrunner/fake/fake.go b/stovepipe/extension/buildrunner/fake/fake.go index b4b82b31..f32b0ed7 100644 --- a/stovepipe/extension/buildrunner/fake/fake.go +++ b/stovepipe/extension/buildrunner/fake/fake.go @@ -71,7 +71,7 @@ func New() buildrunner.BuildRunner { // returns a unique BuildID that encodes the terminal outcome the build should // report at Status time (decided from the headURI marker). baseURI and metadata // are ignored. -func (r runner) Trigger(_ context.Context, headURI, _ string, _ entity.BuildMetadata) (entity.BuildID, error) { +func (r runner) Trigger(_ context.Context, _, headURI string, _ entity.BuildMetadata) (entity.BuildID, error) { outcome := outcomeOK switch marker(headURI) { case tokenTriggerError: diff --git a/stovepipe/extension/buildrunner/fake/fake_test.go b/stovepipe/extension/buildrunner/fake/fake_test.go index f4252bc4..1e2ffe28 100644 --- a/stovepipe/extension/buildrunner/fake/fake_test.go +++ b/stovepipe/extension/buildrunner/fake/fake_test.go @@ -42,7 +42,7 @@ func TestTrigger(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - id, err := New().Trigger(context.Background(), tt.headURI, "", nil) + id, err := New().Trigger(context.Background(), "", tt.headURI, nil) if tt.wantErr { require.Error(t, err) assert.Empty(t, id.ID) @@ -55,9 +55,9 @@ func TestTrigger(t *testing.T) { } func TestTrigger_UniqueIDs(t *testing.T) { - a, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef", "", nil) + a, err := New().Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) require.NoError(t, err) - b, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef", "", nil) + b, err := New().Trigger(context.Background(), "", "git://repo/ref/deadbeef", nil) require.NoError(t, err) assert.NotEqual(t, a.ID, b.ID) } @@ -75,7 +75,7 @@ func TestStatus(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - id, err := New().Trigger(context.Background(), tt.headURI, "", nil) + id, err := New().Trigger(context.Background(), "", tt.headURI, nil) require.NoError(t, err) status, metadata, err := New().Status(context.Background(), id) @@ -98,7 +98,7 @@ func TestStatus_UnrecognizedIDSucceeds(t *testing.T) { } func TestStatus_StatelessAcrossInstances(t *testing.T) { - id, err := New().Trigger(context.Background(), "git://repo/ref/deadbeef?buildrunner-fake=build-fail", "", nil) + id, err := New().Trigger(context.Background(), "", "git://repo/ref/deadbeef?buildrunner-fake=build-fail", nil) require.NoError(t, err) status, _, err := New().Status(context.Background(), id) diff --git a/stovepipe/extension/buildrunner/mock/buildrunner_mock.go b/stovepipe/extension/buildrunner/mock/buildrunner_mock.go index 5b943474..61f1c9d0 100644 --- a/stovepipe/extension/buildrunner/mock/buildrunner_mock.go +++ b/stovepipe/extension/buildrunner/mock/buildrunner_mock.go @@ -73,18 +73,18 @@ func (mr *MockBuildRunnerMockRecorder) Status(ctx, buildID any) *gomock.Call { } // Trigger mocks base method. -func (m *MockBuildRunner) Trigger(ctx context.Context, headURI, baseURI string, metadata entity.BuildMetadata) (entity.BuildID, error) { +func (m *MockBuildRunner) Trigger(ctx context.Context, baseURI, headURI string, metadata entity.BuildMetadata) (entity.BuildID, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Trigger", ctx, headURI, baseURI, metadata) + ret := m.ctrl.Call(m, "Trigger", ctx, baseURI, headURI, metadata) ret0, _ := ret[0].(entity.BuildID) ret1, _ := ret[1].(error) return ret0, ret1 } // Trigger indicates an expected call of Trigger. -func (mr *MockBuildRunnerMockRecorder) Trigger(ctx, headURI, baseURI, metadata any) *gomock.Call { +func (mr *MockBuildRunnerMockRecorder) Trigger(ctx, baseURI, headURI, metadata any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Trigger", reflect.TypeOf((*MockBuildRunner)(nil).Trigger), ctx, headURI, baseURI, metadata) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Trigger", reflect.TypeOf((*MockBuildRunner)(nil).Trigger), ctx, baseURI, headURI, metadata) } // MockFactory is a mock of Factory interface.