test(stovepipe): add build-slow marker to the fake build runner (#464)

## Summary

### Why?

The fake build runner never reports a non-terminal status — `Status`
decides purely from the build id and always returns succeeded, failed,
or an error. That means no integration or e2e stack can reach
`buildsignal`'s reschedule branch, so the entire poll loop is exercised
only by unit tests with mocked publishers. A defect in the reschedule
path is invisible above the unit layer.

### What?

Adds a `build-slow` marker. `Trigger` encodes a ready-at wall-clock
instant into the build id (`fake-build-slow-<readyAtMs>-<suffix>`) and
`Status` reports `running` until that instant passes, then `succeeded`.
Encoding the deadline in the id preserves the fake's stateless,
decide-purely-from-the-id property, so `Trigger` and `Status` can still
live in different processes. The window is a `slowBuildDuration` field
on the runner, defaulted in `New()`, so tests construct a runner with
their own value instead of mutating shared state.

Also widens `marker()` to end a token at `/` as well as `&` and `#`. The
fake SourceControl builds head URIs as `git://<queue>/HEAD`, so a marker
injected through a queue name sits mid-URI and would otherwise be read
as `build-slow/HEAD`. Existing markers sit at the end of a URI and are
unaffected.

No pipeline behaviour changes; this only makes a previously unreachable
path reachable in tests.

## Test Plan

✅ `bazel test //stovepipe/...` — covers `build-slow` reporting running
then succeeded, the no-deadline fallback, and `marker()` against
trailing path segments, query separators, and end-of-URI.



## Stack
1. @ #464
1. #465
1. #466
1. #467
1. #468
1. #469

Author: behinddwalls

stovepipe/extension/buildrunner/fake/fake.go

@@ -14,12 +14,14 @@
 
 // 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
+// as a best-case stub for local-stack/e2e wiring. Other behaviors are injected by
 // embedding a marker token in headURI of the form "buildrunner-fake=<token>":
 //
 //	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
+//	buildrunner-fake=build-slow    -> Status reports BuildStatusRunning for a
+//	                                  short window after Trigger, then succeeds
 //
 // 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
@@ -35,12 +37,20 @@ import (
 	"crypto/rand"
 	"encoding/hex"
 	"fmt"
+	"strconv"
 	"strings"
+	"time"
 
 	"github.com/uber/submitqueue/stovepipe/entity"
 	"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
 )
 
+// defaultSlowBuildDuration is how long a build-slow build reports
+// BuildStatusRunning before succeeding. It must be long enough for the caller's
+// poll loop to observe at least one non-terminal status; tests that need a
+// different window construct a runner with their own value.
+const defaultSlowBuildDuration = 3 * time.Second
+
 // markerPrefix introduces a marker token in headURI: "buildrunner-fake=<token>".
 const markerPrefix = "buildrunner-fake="
 
@@ -49,6 +59,7 @@ const (
 	tokenTriggerError = "trigger-error"
 	tokenFail         = "build-fail"
 	tokenError        = "build-error"
+	tokenSlow         = "build-slow"
 )
 
 // outcomeOK is the BuildID outcome segment for a build that should succeed.
@@ -59,12 +70,17 @@ const outcomeOK = "ok"
 // 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{}
+type runner struct {
+	// slowBuildDuration is how long a build-slow build reports running before
+	// it succeeds. Configuration, not per-build state: it is read at Trigger to
+	// compute the deadline baked into the id, never mutated.
+	slowBuildDuration time.Duration
+}
 
 // New returns a buildrunner.BuildRunner that defaults to succeeding and honors
 // marker tokens embedded in the triggered headURI.
 func New() buildrunner.BuildRunner {
-	return runner{}
+	return runner{slowBuildDuration: defaultSlowBuildDuration}
 }
 
 // Trigger fails when headURI carries the trigger-error marker; otherwise it
@@ -80,6 +96,10 @@ func (r runner) Trigger(_ context.Context, _, headURI string, _ entity.BuildMeta
 		outcome = tokenFail
 	case tokenError:
 		outcome = tokenError
+	case tokenSlow:
+		// A slow build carries the wall-clock instant it becomes terminal, so Status
+		// stays stateless: any instance, in any process, decodes the same deadline.
+		outcome = fmt.Sprintf("%s-%d", tokenSlow, time.Now().Add(r.slowBuildDuration).UnixMilli())
 	}
 
 	// Encode the outcome in the id (e.g. "fake-build-fail-a1b2c3d4") so Status is
@@ -101,6 +121,11 @@ func (r runner) Status(_ context.Context, buildID entity.BuildID) (entity.BuildS
 		return entity.BuildStatusUnknown, nil, fmt.Errorf("fake: marked build error")
 	case strings.Contains(buildID.ID, tokenFail):
 		return entity.BuildStatusFailed, nil, nil
+	case strings.Contains(buildID.ID, tokenSlow):
+		if readyAt, ok := slowReadyAt(buildID.ID); ok && time.Now().UnixMilli() < readyAt {
+			return entity.BuildStatusRunning, nil, nil
+		}
+		return entity.BuildStatusSucceeded, nil, nil
 	default:
 		return entity.BuildStatusSucceeded, nil, nil
 	}
@@ -112,19 +137,38 @@ func (r runner) Cancel(_ context.Context, _ entity.BuildID) error {
 }
 
 // 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.
+// The token ends at the first "&", "#", or "/" delimiter, so a marker may sit
+// among other query parameters, before a fragment, or ahead of a further path
+// segment (as when a URI is built as "git://<queue>/HEAD" and the marker rides
+// in on the queue name).
 func marker(uri string) string {
 	_, rest, found := strings.Cut(uri, markerPrefix)
 	if !found {
 		return ""
 	}
-	if i := strings.IndexAny(rest, "&#"); i >= 0 {
+	if i := strings.IndexAny(rest, "&#/"); i >= 0 {
 		rest = rest[:i]
 	}
 	return rest
 }
 
+// slowReadyAt extracts the epoch-millisecond instant at which a build-slow build
+// becomes terminal, which Trigger encodes into the id as
+// "fake-build-slow-<readyAtMs>-<suffix>". It reports false when the id carries no
+// parsable deadline, in which case the caller treats the build as already terminal.
+func slowReadyAt(id string) (int64, bool) {
+	_, rest, found := strings.Cut(id, tokenSlow+"-")
+	if !found {
+		return 0, false
+	}
+	digits, _, _ := strings.Cut(rest, "-")
+	readyAt, err := strconv.ParseInt(digits, 10, 64)
+	if err != nil {
+		return 0, false
+	}
+	return readyAt, true
+}
+
 // 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.

stovepipe/extension/buildrunner/fake/fake_test.go

@@ -16,7 +16,9 @@ package fake
 
 import (
 	"context"
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -110,3 +112,55 @@ func TestCancel_NoOp(t *testing.T) {
 	err := New().Cancel(context.Background(), entity.BuildID{ID: "anything"})
 	assert.NoError(t, err)
 }
+
+// TestStatus_BuildSlowReportsRunningThenSucceeds covers the one marker that yields a
+// non-terminal status, which is what makes a caller's poll loop reachable in an
+// integration or e2e stack.
+func TestStatus_BuildSlowReportsRunningThenSucceeds(t *testing.T) {
+	// A window long enough that the build is still running when Status is called.
+	slow := runner{slowBuildDuration: 30 * time.Second}
+
+	id, err := slow.Trigger(context.Background(), "", "git://repo/ref/deadbeef?buildrunner-fake=build-slow", nil)
+	require.NoError(t, err)
+
+	status, _, err := New().Status(context.Background(), id)
+	require.NoError(t, err)
+	assert.Equal(t, entity.BuildStatusRunning, status)
+
+	// An id whose deadline has already passed reports the terminal outcome. Encoding
+	// the deadline in the id is what keeps Status stateless across instances.
+	elapsed := entity.BuildID{ID: fmt.Sprintf("fake-build-slow-%d-abcd1234", time.Now().UnixMilli()-1)}
+	status, _, err = New().Status(context.Background(), elapsed)
+	require.NoError(t, err)
+	assert.Equal(t, entity.BuildStatusSucceeded, status)
+}
+
+// TestStatus_BuildSlowWithoutDeadlineSucceeds pins the fallback: an id carrying the
+// marker but no parsable deadline is treated as already terminal rather than polling
+// forever.
+func TestStatus_BuildSlowWithoutDeadlineSucceeds(t *testing.T) {
+	status, _, err := New().Status(context.Background(), entity.BuildID{ID: "fake-build-slow-nodeadline"})
+	require.NoError(t, err)
+	assert.Equal(t, entity.BuildStatusSucceeded, status)
+}
+
+// TestMarker_StopsAtPathSegment covers a marker that arrives mid-URI rather than at the
+// end, as it does when the head URI is built as "git://<queue>/HEAD" and the marker
+// rides in on the queue name.
+func TestMarker_StopsAtPathSegment(t *testing.T) {
+	tests := []struct {
+		name string
+		uri  string
+		want string
+	}{
+		{name: "trailing path segment", uri: "git://repo/main?buildrunner-fake=build-slow/HEAD", want: "build-slow"},
+		{name: "end of uri", uri: "git://repo/ref/deadbeef?buildrunner-fake=build-fail", want: "build-fail"},
+		{name: "query separator", uri: "git://repo/ref?buildrunner-fake=build-fail&other=1", want: "build-fail"},
+		{name: "no marker", uri: "git://repo/ref/deadbeef", want: ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, marker(tt.uri))
+		})
+	}
+}