diff --git a/doc/rfc/stovepipe/steps/buildsignal.md b/doc/rfc/stovepipe/steps/buildsignal.md index e1095acf5..cf0b7f78b 100644 --- a/doc/rfc/stovepipe/steps/buildsignal.md +++ b/doc/rfc/stovepipe/steps/buildsignal.md @@ -1,6 +1,6 @@ # Buildsignal stage -`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, and publishes the **request id** to `record`. See [workflow.md](../workflow.md) for where it sits in the pipeline and [build.md](build.md) for how the build it polls was triggered. +`buildsignal` polls the build-runner until a build reaches a terminal state, records the status, and — once the build is terminal — releases the queue's build slot, projects the outcome onto the `Request`, advances a successful Request's last-green build baseline, and publishes the **request id** to `record`. See [workflow.md](../workflow.md) for where it sits in the pipeline and [build.md](build.md) for how the build it polls was triggered. It handles only the poll loop: it does not decide build strategy, write greenness, or map targets to projects — those are `process`'s, `record`'s, and `analyze`'s jobs. It reports the *fact* (this request's build ended succeeded, failed, or cancelled); `record` derives what that fact *means* for greenness. @@ -60,11 +60,14 @@ For a delivery carrying build id `B`: - with the write-once rule, accepted -> running -> {succeeded|failed|cancelled} is monotonic by mechanism, not by assumption about the backend. -7. If the stored status is terminal, and R does not already carry an outcome: - a. Release the queue's build slot: CAS-decrement Queue.in_flight_count, clamped at zero. +7. If the stored status is terminal: + a. If R does not already carry an outcome, release the queue's build slot: + CAS-decrement Queue.in_flight_count, clamped at zero. - failure here aborts the step: R must not go terminal while still holding a slot. - b. CAS R from processing to the outcome the stored status projects onto it and persist B as R's terminal build id: + b. If R does not already carry an outcome, CAS R from processing to the outcome the stored status projects onto it and persist B as R's terminal build id: succeeded -> succeeded, failed -> failed, cancelled -> cancelled. First writer wins. + c. If the winning durable R.State is succeeded, advance the last-green bookmark + in a CAS retry loop, even on a terminal redelivery. Preserve a newer bookmark. Then publish R.ID to the record topic, partitioned by request id; ack, return. No re-publish to buildsignal. - record loads the Request directly by this key, derives greenness from its outcome, and @@ -84,6 +87,8 @@ For a delivery carrying build id `B`: **Why the slot is released before the outcome write, and why a failed release aborts it**: `Queue` and `Request` are separate entities with no cross-entity transaction, so the ordering picks which crash failure mode we accept. Both rules serve one invariant — *the request must not go terminal while still holding a slot* — because a terminal request is skipped by redelivery and by the DLQ reconciler alike, so nothing would ever decrement it. Failing this way leaves the request non-terminal: redelivery re-runs both steps and decrements again, transiently over-admitting by one slot until the zero clamp reconverges. Over-admission is the failure mode this pipeline already prefers, for the same reason and in the same words as the DLQ reconciler (see [process.md](process.md#in_flight_count-integrity)). +**Baseline timing.** After the winning Request outcome is durable, a successful Request advances `last_green_uri` and `last_green_request_id` in a CAS retry loop before publishing to `record`. Older candidates never regress the bookmark. A terminal redelivery retries the advance without releasing capacity again. Slot release and bookmark advancement are separate updates, so an admission between them can still select the previous baseline. Project resolution, facts, and Git promotion remain in `record`. + **Why `buildsignal` releases the slot rather than `record`**: the gate `process` claims is a *build* slot — it exists to bound concurrent builds per Queue — and once the build is terminal the build is over. Releasing here also keeps the invariant *a terminal `Request` has already released its slot*, which is what makes the DLQ reconciler's early-return on terminal requests safe. **Why `record` hears only terminal signals**: `record` has no non-terminal work — by its own contract a non-terminal signal would be a pure no-op — and step 7 already branches on terminality to decide whether to keep polling, so gating the publish costs nothing and spares `record` a no-op delivery on every poll tick of every running build. Crash-safety is unaffected: a crash between the terminal `Update` and the publish redelivers the message; step 5 re-polls (the runner reports the same terminal status), step 6 no-ops, step 7 publishes. This is a deliberate divergence from SubmitQueue, whose buildsignal republishes to `speculate` on every tick — sound there because speculate is a state machine that may act on any signal; stovepipe has no such consumer. diff --git a/doc/rfc/stovepipe/steps/process.md b/doc/rfc/stovepipe/steps/process.md index 5ca099e6f..b1089fbe0 100644 --- a/doc/rfc/stovepipe/steps/process.md +++ b/doc/rfc/stovepipe/steps/process.md @@ -9,7 +9,7 @@ It handles everything *before* the first build is triggered: it does not run bui Ingest publishes a `ProcessRequest` (request id only — producer and consumer share the store) to the process topic, **partitioned by Queue name**. Two consequences the design relies on: 1. **One partition per Queue**, and `BatchSize = 1` (strict serialization; see [sql-queue-rfc.md](../../sql-queue-rfc.md#strict-serialization-opt-in)) — so at most one `process` invocation runs per Queue at a time. The read-modify-write on the Queue row (gate check, count increment) is therefore race-free **without** a transaction, the same property SubmitQueue's `validate` gets from per-queue partition leasing. -2. **`process` is not the only Queue-row writer.** `ingest` stamps `latest_request_id` (the request id of the newest accepted head, not a URI; see [Backlog coalescing](#backlog-coalescing)) and `buildsignal` releases slots when a build goes terminal, and `record` advances the `last_green_uri` bookmark. They touch different fields under optimistic-locking CAS, so concurrent writes converge instead of clobbering. +2. **`process` is not the only Queue-row writer.** `ingest` stamps `latest_request_id` (the request id of the newest accepted head, not a URI; see [Backlog coalescing](#backlog-coalescing)) and `buildsignal` releases slots when a build goes terminal, then advances the `last_green_uri` bookmark for a successful Request. They touch different fields under optimistic-locking CAS, so concurrent writes converge instead of clobbering. ## Process algorithm @@ -59,7 +59,7 @@ Validation is expensive and shares a baseline, so heads arriving while an earlie | Source | Field | Meaning | |---|---|---| -| Queue row | `last_green_uri` | Bookmark `record` advances on whole-repo green; empty until first green. | +| Queue row | `last_green_uri` | Bookmark `buildsignal` advances on a successful build; empty until first green. | | Queue row | `in_flight_count` | Requests past `process` and not yet terminal. `process` increments on admit; `record` (or DLQ reconciliation) decrements on terminal. | | Queue config | `max_concurrent` | Cap on concurrent in-flight validations. **Default 1** (global wiring default for MVP; per-queue override when a Stovepipe `queueconfig` extension lands). | @@ -80,7 +80,7 @@ The scheme: each admit pins its baseline to last-green *at admit time*; a green Correctness rests on four rules, all with MVP primitives already in place: 1. **Pin to last-green at admit** — a known-green baseline, which `process` already reads. -2. **Advance last-green by ingest order, monotonically** — `record` adopts the highest-counter green (per `CompareRequestID`) and never regresses. (Free at N=1, since verdicts land in order; explicit at N > 1.) +2. **Advance last-green by ingest order, monotonically** — `buildsignal` adopts the highest-counter green (per `CompareRequestID`) and never regresses. (Free at N=1, since verdicts land in order; explicit at N > 1.) 3. **Coalesce to the latest N** instead of the latest 1 — otherwise intermediates are superseded on sight and nothing overlaps. 4. **Fall back to serial on a rewrite** — the linear/superset property is what makes "adopt highest green" sound; rewrites already force a full build with no baseline. @@ -191,7 +191,7 @@ Runtime coordination only — fields the pipeline writes under CAS: | Field | Role | Written by | |---|---|---| | `name` | Stable logical id (`monorepo/main`); the string ingest accepts | ingest (create) | -| `last_green_uri` | Bookmark; empty until first green | record | +| `last_green_uri` | Bookmark; empty until first green | buildsignal | | `in_flight_count` | Active Phase 1 validations | process (+1), record/DLQ (−1) | | `latest_request_id` | Request id of the newest head ingest accepted | ingest | | `version` | Optimistic-locking version | all writers | diff --git a/doc/rfc/stovepipe/steps/record.md b/doc/rfc/stovepipe/steps/record.md index 5d0140027..1a7da8267 100644 --- a/doc/rfc/stovepipe/steps/record.md +++ b/doc/rfc/stovepipe/steps/record.md @@ -2,14 +2,14 @@ `record` turns a terminal build outcome into a durable validation fact. -- Phase 1 records whole-repository greenness. On green it also advances the Queue's last-green bookmark and promotes the commit onto the Queue's promotion ref, then announces the outcome as a hook event. This is implemented: see [stovepipe/controller/record/record.go](../../../../stovepipe/controller/record/record.go). +- Phase 1 records whole-repository greenness and optional project facts. On green it promotes the commit onto the Queue's promotion ref if the Request still holds the last-green bookmark, then announces the outcome as a hook event. This is implemented: see [stovepipe/controller/record/record.go](../../../../stovepipe/controller/record/record.go). - Phase 2 records greenness per project instead of per repository. Sketched here, to be expanded before implementation. Notifying downstream systems rides the cross-domain hook framework rather than a Stovepipe-specific extension; see [Hooks](#hooks). See [workflow.md](../workflow.md) for the whole pipeline, [build.md](build.md) for how builds are created, and [buildsignal.md](buildsignal.md) for the terminal-only handoff into this stage. -`record` owns the validation facts and the two caches derived from them, the last-green bookmark and the promotion ref. It does not decide build scope, poll a build, release the Queue's build slot, interpret a target graph, or map targets to projects. Those belong to `process`, `buildsignal`, and `analyze`. +`record` owns the validation facts and the promotion ref. `buildsignal` advances the last-green build baseline before handing off to this stage. `record` does not decide build scope, poll a build, release the Queue's build slot, interpret a target graph, or map targets to projects. Those belong to `process`, `buildsignal`, and `analyze`. ## Phase 1 algorithm @@ -39,31 +39,30 @@ For a delivery carrying a `Record` payload: - superseded -> ack, no fact. Unreachable in practice. - accepted / processing / anything else -> return a non-retryable invariant error. -6. Map R.State to a whole-repository degree and create the ValidationFact keyed by - (R.URI, empty project) in the queue-bound fact store. +6. Resolve optional project results using R and its TerminalBuildID, and validate + the entire result set before writing facts. A resolver error returns raw. + +7. Map R.State and the project results to a whole-repository degree and create the + ValidationFact keyed by (R.URI, empty project) in the queue-bound fact store. + - succeeded -> DegreeGreen. + - failed with project results -> DegreePartial (0.5). + - failed without project results -> DegreeBroken. - ErrAlreadyExists -> load and reconcile the existing immutable fact. - other store error -> return raw. -7. If the persisted fact is not green, skip to step 10: a break moves neither the - bookmark nor the ref, but is still announced. Report how long it went undetected - first, and only if step 6 is the write that created the fact (see "Observability"). - -8. Advance the bookmark to (R.URI, R.ID) in a CAS retry loop, which also reports - whether R holds the bookmark afterwards: - - stored id empty or older than R.ID -> write under the version guard; R holds it. - - stored id equals R.ID -> R set it on an earlier delivery; no write, - and R still holds it. - - stored id newer -> no write, and R does not hold it. - - ErrVersionMismatch -> reload and re-evaluate. +8. Record each project fact with its resolver-provided degree and persist history + for the repository fact and completed project facts. A failure returns raw; + replay completes missing writes without replacing immutable facts. -9. If R holds the bookmark, ask SourceControl to point the promotion ref at R.URI. - Otherwise skip: whichever commit holds the bookmark owns the ref. - - ErrNotFound -> count and skip. A rewritten history dropped the commit from the - ref, and no retry can promote it. - - other error -> return raw. +9. If the persisted repository fact is not green, skip to step 11. Report how long + the break went undetected only if step 7 created the fact (see "Observability"). + Otherwise load the Queue and check that both LastGreenRequestID and LastGreenURI + still match R. If they do not, skip promotion and continue to step 11. -10. Resolve and record optional project results. - - failure -> return raw; the repository fact, bookmark, and promotion are not undone. +10. Ask SourceControl to point the promotion ref at R.URI. + - ErrNotFound -> count and skip. A rewritten history dropped the commit from the + ref, and no retry can promote it. + - other error -> return raw. 11. Publish a validation.repository.recorded HookEvent naming the request whose fact was persisted. @@ -72,9 +71,9 @@ For a delivery carrying a `Record` payload: 12. ack. ``` -Every decision after step 6 uses the persisted fact, not the outcome read from this delivery's Request. The first immutable fact controls the bookmark, the ref, and the hook event. +Every decision after step 7 uses the persisted fact, not the outcome read from this delivery's Request. The first immutable fact controls the ref and the hook event. -Project-status processing does not delay baseline advancement or promotion. It still completes before the recorded notification. Bookmark advancement and ref promotion remain separate operations; this ordering does not make them atomic. +Project-status processing delays fact creation and promotion, while the build baseline is already available from `buildsignal`. All fact writes and history complete before promotion and the recorded notification. The fact writes, bookmark advancement, and ref promotion remain separate operations. ## Validation facts @@ -113,25 +112,26 @@ The fail-closed path is subtler than "no fact". A DLQ reconciler forces the Requ ### Phase 1 degree mapping -Whole-repository builds use only the endpoints, mapped from the outcome `buildsignal` stamped on the Request: +Whole-repository degree depends on the outcome `buildsignal` stamped on the Request and whether project results are available: | Request outcome | Result | | --------------- | ----------------------------- | | `succeeded` | fact at `entity.DegreeGreen` | -| `failed` | fact at `entity.DegreeBroken` | +| `failed`, project results available | fact at `entity.DegreePartial` (`0.5`) | +| `failed`, no project results | fact at `entity.DegreeBroken` (`1`) | | `cancelled` | no fact | -Intermediate degrees are reserved for project analysis and deferred with the project mapping contract. Phase 1 does not manufacture fractional values. +`DegreePartial` is hard coded to `0.5` for now; it does not measure the fraction of projects failing. Each project fact preserves the degree returned by the resolver. ## Last-green advancement (Queue bookmark) -The bookmark only moves forward. `entity.Queue` carries `LastGreenURI` alongside `LastGreenRequestID`, the request id that owns the current URI, plus `InFlightCount` and `LatestRequestID`. The guard compares ids with `entity.CompareRequestID(R.Queue, …)`, the same ingest-order comparison `ingest` and `process` use for coalescing. It errors on an id that does not match the queue's format, which is non-retryable: re-parsing the same ids cannot start succeeding. +`buildsignal` advances the bookmark after releasing the build slot and persisting the successful Request outcome, before publishing to `record`. The bookmark only moves forward. `entity.Queue` carries `LastGreenURI` alongside `LastGreenRequestID`, the request id that owns the current URI, plus `InFlightCount` and `LatestRequestID`. The guard compares ids with `entity.CompareRequestID(R.Queue, …)`, the same ingest-order comparison `ingest` and `process` use for coalescing. It errors on an id that does not match the queue's format, which is non-retryable: re-parsing the same ids cannot start succeeding. The write goes through `QueueStore.Update(ctx, queue, oldVersion, newVersion)`, so version arithmetic stays in the controller: it computes `newVersion = oldVersion + 1` and the store does a pure conditional write. `ErrVersionMismatch` is absorbed by the loop rather than returned, since a concurrent writer moved the row and reloading converges. -**Why the bookmark moves only after the fact is durable.** The bookmark is a cache of "newest green URI" derived from the facts, so it must never lead them. Losing the advance to a crash is recoverable, since the redelivery reloads the same fact and retries. A bookmark with no fact behind it would point at greenness nothing recorded, and `process` would pick a baseline URI that no validation supports. +The bookmark is a successful-build baseline and can precede validation facts. Project resolution and Git promotion remain in `record`. Releasing capacity and advancing the bookmark are separate Queue updates, so a new build may be admitted with the previous baseline before the advance lands. A terminal redelivery retries the bookmark update without releasing the slot again. A failed or cancelled build never moves the bookmark, and so never promotes either. @@ -139,7 +139,7 @@ A failed or cancelled build never moves the bookmark, and so never promotes eith A green commit is also promoted: `record` asks `SourceControl` to point the Queue's promotion ref, a stable branch name like `verified-main`, at that commit. This is the pull-shaped counterpart to the hook event's push, so a deploy gate or cache warmer can resolve a name and know nothing about Stovepipe, URIs, or degrees. See [Promotion ref](../workflow.md#promotion-ref--the-last-green-commit-by-name) in the pipeline overview. The stage names only the commit; which ref a Queue promotes to, and whether it has one at all, is integrator configuration injected into the `SourceControl` implementation alongside the endpoint and credentials, so a backend with no promotion target makes `Promote` a no-op. -**Promotion is gated on holding the bookmark**, and only the holder promotes. Without that gate an older green commit recording late would drag the ref backwards: the ref has no version guard of its own, and `Promote` lands the URI it is given rather than refusing a non-descendant, so the caller has to enforce monotonicity. Deciding it from the bookmark instead of a fresh comparison means both caches move on one already-serialized decision. +**Promotion is gated on holding the bookmark**, and only the holder promotes. Without that gate an older green commit recording late would drag the ref backwards: the ref has no version guard of its own, and `Promote` lands the URI it is given rather than refusing a non-descendant, so the caller has to enforce monotonicity. The stored Request id and URI must both match before promotion. The Queue read and external Git write remain separate operations. The equal case matters as much as the newer one. A redelivery that finds its own id on the bookmark reports that it *holds* it rather than skipping, so the promotion behind it is retried. That is what makes a crash between the bookmark write and the promotion recoverable, and it is safe because `Promote` is idempotent. Promotion follows the durable fact for the same reason the bookmark does: the ref is a cache of the facts and must not lead them. @@ -280,7 +280,7 @@ Every primary subscription comes from `DefaultSubscriptionConfig`, which enables Two different things put a message there, and only one is a poison payload. A delivery that fails with its retry budget spent is dead-lettered by the nack itself, carrying the reason it actually failed. A delivery that never reaches a nack, because it crashed or because its **ack failed** and the visibility timeout redelivered it, is dead-lettered by the poll loop once `retry_count` reaches `MaxAttempts` (3 by default), without the controller running on that final attempt and with only a generic reason recorded. So a missing reconciler exposes more than malformed messages: a fact can be lost to a storage failure that would have succeeded on a later retry, or to an ack that never landed even though the write did. -Gating stays safe, because everything this stage can lose reads as not-green: a Request with no fact is indistinguishable from one not yet validated. What is lost is the *fact*. A green build whose fact write permanently failed leaves the URI looking unvalidated, which costs the queue an incremental baseline and forces a full build at the next head. A lost notification joins that list, and unlike the fact it gets no second chance from a later commit. +Gating stays safe, because everything this stage can lose reads as not-green: a Request with no fact is indistinguishable from one not yet validated. What is lost is the *fact*. A green build whose fact write permanently failed leaves the URI looking unvalidated, while its successful-build baseline remains available to the next head. A lost notification joins that list, and unlike the fact it gets no second chance from a later commit. The reconciler does not re-run the stage. It loads the Request only to identify existing durable state and retain a stable `record_abandoned` event. The DLQ consumer uses `errs.AlwaysRetryableProcessor`, so a transient Request read or history persistence failure keeps retrying until that observable abandonment is durable. A malformed payload, invalid or unresolvable queue identity, or missing Request cannot be repaired by redelivery; those cases are logged, counted, and acknowledged so poison cannot occupy the DLQ indefinitely. No DLQ path creates a fact, advances a bookmark, invokes source control, or publishes a hook. diff --git a/doc/rfc/stovepipe/workflow.md b/doc/rfc/stovepipe/workflow.md index 4383a0175..3d54e7c9c 100644 --- a/doc/rfc/stovepipe/workflow.md +++ b/doc/rfc/stovepipe/workflow.md @@ -100,25 +100,25 @@ What runs is one pass per Request. It establishes whole-repository greenness, an │ buildsignal │ │ Poll until terminal; record │ │ status; release the slot; │ - │ project the outcome │ + │ save outcome; advance green │ └───────────────┬──────────────┘ │ RequestID ▼ ┌──────────────────────────────┐ Hooks │ record │┄┄┄┄┄► "URI green, - │ Write whole-repo greenness; │ not green, or - │ on green advance last-green │ cancelled" - │ and the promotion ref; │ - │ resolve project results │ - │ inline │ + │ Resolve project results; │ not green, or + │ record facts; on green move │ cancelled" + │ the promotion ref; │ + │ announce recorded outcome │ + │ │ └──────────────────────────────┘ ``` 1. **ingest** — invoked by the external poller with a **Queue name**. It asks `SourceControl` for that Queue's current head URI, mints a Request namespaced by the Queue, persists it with no recorded greenness yet, and dedups on `(Queue, head URI)` so a re-reported head is processed once. It publishes the RequestID onward. 2. **process** — decides build strategy (incremental since last-green vs full monorepo), gates concurrent work per Queue, coalesces backlog to the latest head, publishes a **hook event** announcing that validation of the commit has begun, and publishes to `build`. See [process.md](steps/process.md). 3. **build** — runs the build-runner for the chosen scope. A flag derived from `process` decides whether to build relative to the last-green **baseline URI** (incremental) or from scratch (full). It records a build and publishes the BuildID. -4. **buildsignal** — polls until the build is terminal, records that status, releases the Queue's `in_flight_count` slot, projects the terminal status and build id onto the Request (`succeeded` / `failed` / `cancelled`), and publishes the RequestID to `record`. -5. **record** — for a succeeded or failed Request, writes the whole-repo greenness for the head URI (`0` green / `1` broken to start), derived from the Request's build outcome. On green it advances the Queue's **last-green URI** so the next `process` can build incrementally from here, and asks `SourceControl` to advance the Queue's **promotion ref** to the same commit (see [Promotion ref](#promotion-ref--the-last-green-commit-by-name)). It then gives the Request's terminal build id to `projectstatus.Resolver` and writes one validation fact per returned result. The example server uses the noop resolver, so that list is empty unless a deployment supplies another. It publishes `validation.repository.recorded` for that fact. The Queue's `in_flight_count` was already released by `buildsignal` when the build went terminal. A cancelled Request writes no fact and publishes `validation.repository.cancelled`, so a consumer can stop waiting on the commit. +4. **buildsignal** — polls until terminal, records the status, releases the build slot, persists the winning Request outcome, and advances a successful-build baseline before publishing the RequestID to `record`. Slot release and baseline advancement are separate updates, so the next admission may still select the previous baseline. +5. **record** — resolves project results, then records repository degree `0` for success, `0.5` for failure with project results, or `1` for failure without results. Project facts preserve their resolved degrees. Once facts and history are durable, a green fact holding the current baseline promotes in Git; the recorded hook follows. A cancelled Request writes no fact and publishes `validation.repository.cancelled`. ### Designed, not built: project analysis @@ -131,8 +131,8 @@ An `analyze` stage is not part of the pipeline. `stovepipe/core/messagequeue` ha | **ingest** | Queue name (from poller) | process | Resolve head URI via SourceControl, mint Request, persist (no greenness), dedup on `(Queue, head URI)` | | **process** | RequestID | build, hook topic | Build strategy, concurrency gate, backlog coalescing; announce validation start on admit → [process.md](steps/process.md) | | **build** | RequestID | buildsignal | Run the build-runner for the chosen scope; baseline = last-green URI iff incremental | -| **buildsignal** | BuildID | record | Record terminal build status; release `in_flight_count`; project the outcome onto the Request; publish the request id | -| **record** | RequestID | hook topic | Write whole-repo greenness; resolve project results inline; on green advance last-green URI and the promotion ref; publish the recorded or cancelled repository hook event | +| **buildsignal** | BuildID | record, buildanalysis | Poll until terminal, release capacity, persist the outcome, advance a successful baseline, and publish downstream work | +| **record** | RequestID | hook topic | Resolve projects, record repository and project facts, promote a green baseline holder, and publish the repository hook | | **hook** | HookEvent | — | Invoke the hooks the resolver returns. The example server resolves every event to noop | ## Step RFCs diff --git a/stovepipe/controller/buildsignal/buildsignal.go b/stovepipe/controller/buildsignal/buildsignal.go index 32ceb7be4..c6d3e0f30 100644 --- a/stovepipe/controller/buildsignal/buildsignal.go +++ b/stovepipe/controller/buildsignal/buildsignal.go @@ -15,8 +15,9 @@ // Package buildsignal holds the buildsignal-stage queue controller. It consumes // BuildSignal messages (a build id), polls the build-runner until the build // reaches a terminal status, persists that status, and — once terminal — -// releases the queue's build slot, projects the outcome onto the request, and -// publishes directly to record plus terminal analysis. See +// releases the queue's build slot, projects the outcome onto the request, +// advances the successful-build bookmark, and publishes directly to record plus +// terminal analysis. See // doc/rfc/stovepipe/steps/buildsignal.md. package buildsignal @@ -178,6 +179,12 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er if err := c.finishRequest(ctx, store, &request, build.ID, effective); err != nil { return err } + if request.State == entity.RequestStateSucceeded { + if err := c.advanceLastGreen(ctx, store, request); err != nil { + metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) + return err + } + } if err := c.persistOutcomeLog(ctx, store, request); err != nil { return err } @@ -360,6 +367,62 @@ func (c *Controller) releaseBuildSlot(ctx context.Context, store storage.Storage } } +// advanceLastGreen points the queue's bookmark at the successful request, retrying +// on version conflicts. The bookmark only moves forward and can precede facts. +// Capacity is already released, so another admission may use the prior baseline. +func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage, request entity.Request) error { + queueStore := store.GetQueueStore() + + for { + queueRow, err := queueStore.Get(ctx, request.Queue) + if err != nil { + return fmt.Errorf("failed to load queue %s to advance last green: %w", request.Queue, err) + } + + cmp, err := compareToBookmark(request.Queue, request.ID, queueRow.LastGreenRequestID) + if err != nil { + // Non-retryable: re-parsing the same ids cannot start succeeding. + return err + } + if cmp <= 0 { + return nil + } + + updated := queueRow + updated.LastGreenURI = request.URI + updated.LastGreenRequestID = request.ID + newVersion := queueRow.Version + 1 + if err := queueStore.Update(ctx, updated, queueRow.Version, newVersion); err != nil { + if errors.Is(err, storage.ErrVersionMismatch) { + continue + } + return fmt.Errorf("failed to advance last green for queue %s: %w", request.Queue, err) + } + + metrics.NamedCounter(c.metricsScope, _opName, "last_green_advanced", 1, metrics.TagsFromContext(ctx)...) + c.logger.Infow("advanced last green bookmark", + "queue", request.Queue, + "request_id", request.ID, + "last_green_uri", request.URI, + ) + return nil + } +} + +// compareToBookmark orders candidate against the request id currently holding the +// bookmark, by ingest order, using the sign convention of entity.CompareRequestID. +// An empty current means the bookmark has never been set, so any candidate is newer. +func compareToBookmark(queue, candidate, current string) (int, error) { + if current == "" { + return 1, nil + } + cmp, err := entity.CompareRequestID(candidate, current) + if err != nil { + return 0, fmt.Errorf("failed to compare request ids for queue %s: %w", queue, err) + } + return cmp, nil +} + // reconcile persists a real status transition and returns the status that // should drive the rest of Process: the polled status when persisted (or // already unchanged), or the stored status when a stored terminal status is diff --git a/stovepipe/controller/buildsignal/buildsignal_test.go b/stovepipe/controller/buildsignal/buildsignal_test.go index d789f839e..5731831d7 100644 --- a/stovepipe/controller/buildsignal/buildsignal_test.go +++ b/stovepipe/controller/buildsignal/buildsignal_test.go @@ -188,7 +188,22 @@ func expectFinishWrites(m buildsignalMocks, state entity.RequestState) *gomock.C eventCall := expectBuildFinished(m) m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(1, 4), nil).After(eventCall) m.queueStore.EXPECT().Update(gomock.Any(), queueRow(0, 4), int32(4), int32(5)).Return(nil) - return m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(state), int32(1), int32(2)).Return(nil) + outcomeCall := m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(state), int32(1), int32(2)).Return(nil) + if state == entity.RequestStateSucceeded { + return expectAdvanceLastGreen(m, outcomeCall) + } + return outcomeCall +} + +func expectAdvanceLastGreen(m buildsignalMocks, after *gomock.Call) *gomock.Call { + getCall := m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(0, 5), nil) + if after != nil { + getCall.After(after) + } + updated := queueRow(0, 5) + updated.LastGreenURI = requestWithState(entity.RequestStateSucceeded).URI + updated.LastGreenRequestID = testID + return m.queueStore.EXPECT().Update(gomock.Any(), updated, int32(5), int32(6)).Return(nil).After(getCall) } func expectBuildFinished(m buildsignalMocks) *gomock.Call { @@ -380,9 +395,13 @@ func TestProcess(t *testing.T) { m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil) m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil) // No build/queue/request write: the status is unchanged and the outcome - // is already recorded, so the slot must not be released a second time. + // and bookmark are already recorded, so the slot must not be released a second time. eventCall := expectBuildFinished(m) - logCall := expectOutcomeLog(m, entity.RequestStateSucceeded, 2).After(eventCall) + queue := queueRow(1, 6) + queue.LastGreenRequestID = testID + queue.LastGreenURI = request.URI + getCall := m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queue, nil).After(eventCall) + logCall := expectOutcomeLog(m, entity.RequestStateSucceeded, 2).After(getCall) m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil).After(logCall) m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil).After(logCall) }, @@ -419,13 +438,14 @@ func TestProcess(t *testing.T) { m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil) m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil) eventCall := expectBuildFinished(m) + bookmarkCall := expectAdvanceLastGreen(m, eventCall) log := requestlog.NewRequestStateLog(request, entity.RequestOutcomeReasonBuildSucceeded) log.Metadata[requestlog.MetadataKeyBuildID] = request.TerminalBuildID m.materializer.EXPECT().PersistLog( gomock.Any(), m.store, log, - ).Return(errors.New("db down")).After(eventCall) + ).Return(errors.New("db down")).After(bookmarkCall) }, }, { @@ -697,3 +717,96 @@ func TestNonterminalPollDoesNotPublishTerminalAnalysis(t *testing.T) { require.NoError(t, c.Process(queueContext(), d)) } + +func TestProcessRetriesBookmarkWithoutReleasingSlotAgain(t *testing.T) { + for _, committed := range []bool{false, true} { + t.Run(map[bool]string{false: "write failed", true: "write committed but response lost"}[committed], func(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + request := requestWithState(entity.RequestStateSucceeded) + m.buildStore.EXPECT().Get(gomock.Any(), testBuildID).Return(build(entity.BuildStatusSucceeded, 3), nil).Times(2) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil).Times(2) + m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil).Times(2) + m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil).Times(2) + m.materializer.EXPECT().PersistLog(gomock.Any(), m.store, gomock.Any()).Return(nil).Times(3) + queue := queueRow(1, 5) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).DoAndReturn(func(context.Context, string) (entity.Queue, error) { + return queue, nil + }).Times(2) + m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(5), int32(6)).DoAndReturn( + func(_ context.Context, updated entity.Queue, _, version int32) error { + assert.Equal(t, int32(1), updated.InFlightCount) + assert.Equal(t, request.ID, updated.LastGreenRequestID) + assert.Equal(t, request.URI, updated.LastGreenURI) + if committed { + queue = updated + queue.Version = version + } + return driver.ErrBadConn + }) + require.ErrorIs(t, c.Process(queueContext(), delivery(t, ctrl, buildSignalPayload(t, testBuildID))), driver.ErrBadConn) + if !committed { + updated := queue + updated.LastGreenRequestID = request.ID + updated.LastGreenURI = request.URI + m.queueStore.EXPECT().Update(gomock.Any(), updated, int32(5), int32(6)).Return(nil) + } + m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil) + m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil) + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, buildSignalPayload(t, testBuildID)))) + }) + } +} + +func TestProcessBookmarkPreservesConcurrentQueueUpdates(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + request := requestWithState(entity.RequestStateSucceeded) + m.buildStore.EXPECT().Get(gomock.Any(), testBuildID).Return(build(entity.BuildStatusSucceeded, 3), nil) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil) + m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil) + m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(entity.BuildStatusSucceeded, nil, nil) + m.materializer.EXPECT().PersistLog(gomock.Any(), m.store, gomock.Any()).Return(nil).Times(2) + stale := queueRow(0, 5) + fresh := queueRow(1, 6) + fresh.LastGreenRequestID = "9" + fresh.LastGreenURI = "newer-commit" + gomock.InOrder( + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(stale, nil), + m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(5), int32(6)).Return(storage.ErrVersionMismatch), + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(fresh, nil), + ) + m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil) + m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil) + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, buildSignalPayload(t, testBuildID)))) +} + +func TestProcessBookmarkUsesWinningRequestOutcome(t *testing.T) { + for _, state := range []entity.RequestState{entity.RequestStateSucceeded, entity.RequestStateFailed} { + t.Run(string(state), func(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + status := entity.BuildStatusSucceeded + if state == entity.RequestStateSucceeded { + status = entity.BuildStatusFailed + } + m.buildStore.EXPECT().Get(gomock.Any(), testBuildID).Return(build(status, 3), nil) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateProcessing), nil) + m.runnerFactory.EXPECT().For(buildrunner.Config{QueueName: testQueue}).Return(m.runner, nil) + m.runner.EXPECT().Status(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(status, nil, nil) + m.materializer.EXPECT().PersistLog(gomock.Any(), m.store, gomock.Any()).Return(nil).Times(2) + gomock.InOrder( + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(1, 4), nil), + m.queueStore.EXPECT().Update(gomock.Any(), queueRow(0, 4), int32(4), int32(5)).Return(nil), + m.reqStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(storage.ErrVersionMismatch), + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(state), nil), + ) + if state == entity.RequestStateSucceeded { + expectAdvanceLastGreen(m, nil) + } + m.publisher.EXPECT().Publish(gomock.Any(), "record", gomock.Any()).Return(nil) + m.publisher.EXPECT().Publish(gomock.Any(), "buildanalysis", gomock.Any()).Return(nil) + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, buildSignalPayload(t, testBuildID)))) + }) + } +} diff --git a/stovepipe/controller/project_status_test.go b/stovepipe/controller/project_status_test.go index 6832c7152..97d8017a7 100644 --- a/stovepipe/controller/project_status_test.go +++ b/stovepipe/controller/project_status_test.go @@ -70,6 +70,7 @@ func TestGetProjectStatus(t *testing.T) { wantConsistency bool }{ {name: "in progress without fact", request: entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-a"}}, factErr: storage.ErrNotFound}, + {name: "recorded partial repository fact", request: entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI}, fact: entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Degree: entity.DegreePartial}, wantFact: true}, {name: "recorded green fact", request: entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-a"}}, fact: entity.ValidationFact{URI: projectStatusURI, RequestID: projectStatusID, Degree: entity.DegreeGreen}, wantFact: true}, {name: "missing uri", request: entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-a"}}, uriErr: storage.ErrNotFound, wantNotFound: true}, {name: "request summary visibility gap", request: entity.GetProjectStatusRequest{Queue: projectStatusQueue, ChangeURI: projectStatusURI, Projects: []string{"project-a"}}, summaryErr: storage.ErrNotFound, wantRetryable: true}, diff --git a/stovepipe/controller/record/record.go b/stovepipe/controller/record/record.go index 2b6ef37df..8e9dff5f7 100644 --- a/stovepipe/controller/record/record.go +++ b/stovepipe/controller/record/record.go @@ -16,10 +16,11 @@ // messages (a request id) published by buildsignal once a build reaches a // terminal status, and turns that outcome into durable validation state. // -// The durable state is a ValidationFact per validated commit, plus the queue's -// last-green bookmark, which process reads to choose an incremental build -// baseline. A green commit is also promoted, moving the queue's promotion ref so -// downstream systems can pull the latest green commit by name. +// The durable state is a ValidationFact per validated commit and project. Build +// completion owns the queue's last-green bookmark, which process reads to choose +// an incremental build baseline. A green commit is also promoted after its facts +// are persisted, moving the queue's promotion ref so downstream systems can pull +// the latest green commit by name. // // The outcome is then announced as a hook event, so integrations outside the // pipeline learn that a commit is green or broken. See @@ -55,7 +56,7 @@ import ( ) // Controller consumes Record messages, records the build's validation fact, and -// when that fact is green advances the queue's last-green bookmark and promotes +// when that fact is green and holds the queue's last-green bookmark promotes // the commit. Implements consumer.Controller. type Controller struct { logger *zap.SugaredLogger @@ -147,17 +148,21 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er switch request.State { case entity.RequestStateSucceeded, entity.RequestStateFailed: - fact, created, err := c.recordFact(ctx, store, request) + results, err := c.resolveProjectResults(ctx, request) + if err != nil { + return err + } + fact, created, err := c.recordFact(ctx, store, request, results) if err != nil { return err } if err := c.persistValidationFactRecordedLog(ctx, store, request, fact); err != nil { return err } - if err := c.applyFactToDerivedCaches(ctx, store, request, fact, created); err != nil { + if err := c.persistProjectFacts(ctx, store, request, results); err != nil { return err } - if err := c.recordProjectFacts(ctx, store, request); err != nil { + if err := c.applyFactToDerivedCaches(ctx, store, request, fact, created); err != nil { return err } return c.publishHookEvent(ctx, request, hookevent.NewValidationRepositoryRecorded(request)) @@ -183,19 +188,23 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er } } -func (c *Controller) recordProjectFacts(ctx context.Context, store storage.Storage, request entity.Request) error { +func (c *Controller) resolveProjectResults(ctx context.Context, request entity.Request) ([]projectstatus.Result, error) { resolver, err := c.projectStatusResolverFactory.For(projectstatus.ResolverConfig{QueueName: request.Queue}) if err != nil { - return fmt.Errorf("resolve project status resolver for queue %q: %w", request.Queue, err) + return nil, fmt.Errorf("resolve project status resolver for queue %q: %w", request.Queue, err) } results, err := resolver.Resolve(ctx, request, request.TerminalBuildID) if err != nil { - return fmt.Errorf("resolve project status results for request %q: %w", request.ID, err) + return nil, fmt.Errorf("resolve project status results for request %q: %w", request.ID, err) } if err := validateProjectStatusResults(request, results); err != nil { - return err + return nil, err } + return results, nil +} + +func (c *Controller) persistProjectFacts(ctx context.Context, store storage.Storage, request entity.Request, results []projectstatus.Result) error { for _, result := range results { _, _, err := c.recordValidationFact(ctx, store.GetValidationFactStore(), entity.ValidationFact{ URI: request.URI, @@ -270,10 +279,10 @@ func (c *Controller) persistValidationFactRecordedLog( return nil } -// applyFactToDerivedCaches moves the two caches that follow the persisted fact: a -// green fact advances the queue's bookmark and, when this request ends up holding -// it, promotes the commit. A broken fact moves neither, and instead reports how -// long the break it names went undetected. +// applyFactToDerivedCaches follows the persisted fact: a green fact promotes the +// commit when this request holds the queue's bookmark, set by build completion. +// A broken fact skips promotion, and instead reports how long the break it names +// went undetected. func (c *Controller) applyFactToDerivedCaches( ctx context.Context, store storage.Storage, @@ -292,32 +301,38 @@ func (c *Controller) applyFactToDerivedCaches( return nil } - holdsBookmark, err := c.advanceLastGreen(ctx, store, request) + queue, err := store.GetQueueStore().Get(ctx, request.Queue) if err != nil { metrics.NamedCounter(c.metricsScope, _opName, "storage_errors", 1, metrics.TagsFromContext(ctx)...) return err } - if !holdsBookmark { - // A later green commit already holds the bookmark, so it also owns the - // promotion ref: promoting this older commit would move the ref backwards. + if queue.LastGreenRequestID != "" { + if _, err := entity.CompareRequestID(request.ID, queue.LastGreenRequestID); err != nil { + return err + } + } + if queue.LastGreenRequestID != request.ID || queue.LastGreenURI != request.URI { + // Only the current baseline owner can promote: promoting an older commit + // would move the ref backwards. return nil } + c.emitLastGreenTimestamp(ctx, request) return c.promote(ctx, request) } // recordFact writes the request's outcome as an immutable whole-repository fact and // returns the fact that is actually stored, which is not always the one just built: // facts are first-writer-wins, so an identity already claimed by this same request — -// a redelivery after the write but before the bookmark advanced — yields the stored +// a redelivery after the write but before promotion — yields the stored // fact instead. Every decision downstream reads that stored fact rather than the // request, so a redelivery cannot reach a different verdict than the original. The // second return reports whether this call is the one that wrote the fact, which is // how a caller tells the original delivery from a redelivery. -func (c *Controller) recordFact(ctx context.Context, store storage.Storage, request entity.Request) (entity.ValidationFact, bool, error) { +func (c *Controller) recordFact(ctx context.Context, store storage.Storage, request entity.Request, results []projectstatus.Result) (entity.ValidationFact, bool, error) { return c.recordValidationFact(ctx, store.GetValidationFactStore(), entity.ValidationFact{ URI: request.URI, Project: wholeRepositoryProject, - Degree: degreeFor(request.State), + Degree: degreeFor(request.State, results), RequestID: request.ID, CreatedAt: time.Now().UnixMilli(), }) @@ -429,67 +444,16 @@ func (c *Controller) failureDetectionUnobserved(ctx context.Context, request ent ) } -// degreeFor maps a request's build outcome onto a whole-repository degree. Only the -// endpoints are produced: a whole-repository build is either clean or it is not, and -// intermediate degrees need per-project attribution that this stage does not do. -func degreeFor(state entity.RequestState) float64 { +// degreeFor maps a request's build outcome onto a whole-repository degree. +// A failed build with project results has a partial degree. +func degreeFor(state entity.RequestState, results []projectstatus.Result) float64 { if state == entity.RequestStateSucceeded { return entity.DegreeGreen } - return entity.DegreeBroken -} - -// advanceLastGreen points the queue's bookmark at request, retrying on version -// conflicts, and reports whether request holds the bookmark afterwards. The -// bookmark only moves forward: an older candidate is skipped without a write and -// does not hold it, while a redelivery of the request that already set it holds it -// without a write, so the promotion that follows is retried. -// -// The bookmark is a cache of "newest green URI" derived from the facts, so it is -// advanced only after the green fact is durable. Losing the advance to a crash is -// recoverable — the redelivery reloads the same fact and retries — whereas a -// bookmark with no fact behind it would point at greenness nothing recorded. -func (c *Controller) advanceLastGreen(ctx context.Context, store storage.Storage, request entity.Request) (bool, error) { - queueStore := store.GetQueueStore() - - for { - queueRow, err := queueStore.Get(ctx, request.Queue) - if err != nil { - return false, fmt.Errorf("failed to load queue %s to advance last green: %w", request.Queue, err) - } - - cmp, err := compareToBookmark(request.Queue, request.ID, queueRow.LastGreenRequestID) - if err != nil { - // Non-retryable: re-parsing the same ids cannot start succeeding. - return false, err - } - if cmp < 0 { - return false, nil - } - if cmp == 0 { - return true, nil - } - - updated := queueRow - updated.LastGreenURI = request.URI - updated.LastGreenRequestID = request.ID - newVersion := queueRow.Version + 1 - if err := queueStore.Update(ctx, updated, queueRow.Version, newVersion); err != nil { - if errors.Is(err, storage.ErrVersionMismatch) { - continue - } - return false, fmt.Errorf("failed to advance last green for queue %s: %w", request.Queue, err) - } - - metrics.NamedCounter(c.metricsScope, _opName, "last_green_advanced", 1, metrics.TagsFromContext(ctx)...) - c.logger.Infow("advanced last green bookmark", - "queue", request.Queue, - "request_id", request.ID, - "last_green_uri", request.URI, - ) - c.emitLastGreenTimestamp(ctx, request) - return true, nil + if len(results) > 0 { + return entity.DegreePartial } + return entity.DegreeBroken } // emitLastGreenTimestamp emits the creation time of the change the bookmark now @@ -547,7 +511,7 @@ func (c *Controller) emitLastGreenTimestamp(ctx context.Context, request entity. // the queue has one at all — is source-control configuration, so this stage names // only the commit. // -// Like the bookmark, the ref is a cache of the facts, so it moves only after the +// The ref is a cache of the facts, so it moves only after the // green fact is durable. Promotion is idempotent, so a redelivery repeats it // harmlessly. A commit that a rewritten history dropped from the ref cannot be // promoted by any retry, so that case is counted and skipped rather than failed. @@ -616,20 +580,6 @@ func (c *Controller) publishHookEvent(ctx context.Context, request entity.Reques return nil } -// compareToBookmark orders candidate against the request id currently holding the -// bookmark, by ingest order, using the sign convention of entity.CompareRequestID. -// An empty current means the bookmark has never been set, so any candidate is newer. -func compareToBookmark(queue, candidate, current string) (int, error) { - if current == "" { - return 1, nil - } - cmp, err := entity.CompareRequestID(candidate, current) - if err != nil { - return 0, fmt.Errorf("failed to compare request ids for queue %s: %w", queue, err) - } - return cmp, nil -} - // loadRequest loads the request by id. func (c *Controller) loadRequest(ctx context.Context, store storage.Storage, id string) (entity.Request, error) { return loader.ByID(ctx, id, store.GetRequestStore().Get, "request") diff --git a/stovepipe/controller/record/record_test.go b/stovepipe/controller/record/record_test.go index 0743ef77a..31e2f4539 100644 --- a/stovepipe/controller/record/record_test.go +++ b/stovepipe/controller/record/record_test.go @@ -275,88 +275,31 @@ func queueRow(lastGreenURI, lastGreenRequestID string, version int32) entity.Que } } -func TestProcess_AdvancesBookmarkOnSuccess(t *testing.T) { - tests := []struct { - name string - stored entity.Queue - wantURI string - }{ - { - name: "bookmark unset", - stored: queueRow("", "", 1), - wantURI: testURI, - }, - { - name: "stored bookmark is older", - stored: queueRow("git://remote/monorepo/main/old", "3", 4), - wantURI: testURI, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctrl := gomock.NewController(t) - c, m := newController(t, ctrl) - - m.reqStore.EXPECT().Get(gomock.Any(), testID). - Return(requestWithState(entity.RequestStateSucceeded), nil) - var fact entity.ValidationFact - m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(tt.stored, nil) - - var written entity.Queue - m.queueStore.EXPECT(). - Update(gomock.Any(), gomock.Any(), tt.stored.Version, tt.stored.Version+1). - DoAndReturn(func(_ context.Context, q entity.Queue, _, _ int32) error { - written = q - return nil - }) - m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). - Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) - m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) - - require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) - assert.Equal(t, testQueue, m.hooks.tenant) - assert.Equal(t, tt.wantURI, written.LastGreenURI) - assert.Equal(t, testID, written.LastGreenRequestID) - - gauge, ok := m.metricsScope.Snapshot().Gauges()["record_controller.record.last_green_timestamp_seconds+queue=monorepo/main"] - require.True(t, ok) - assert.Equal(t, float64(testChangeTime.Unix()), gauge.Value()) - - // The green fact is what authorises the advance. - assert.Equal(t, entity.DegreeGreen, fact.Degree) - assert.Equal(t, testURI, fact.URI) - assert.Equal(t, testID, fact.RequestID) - assert.Equal(t, wholeRepositoryProject, fact.Project) - assert.Positive(t, fact.CreatedAt) - }) - } +func TestProcess_PromotesFinalizedGreenAtCurrentBaseline(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateSucceeded), nil) + var fact entity.ValidationFact + m.expectFactCreated(&fact) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) + m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) + assert.Equal(t, entity.DegreeGreen, fact.Degree) + assert.Len(t, m.hooks.events, 1) } -func TestProcess_PromotesGreenBeforeProjectResolution(t *testing.T) { +func TestProcess_WaitsForProjectResolutionBeforeRecordingOrPromotion(t *testing.T) { ctrl := gomock.NewController(t) c, m := newController(t, ctrl) request := requestWithState(entity.RequestStateSucceeded) m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil) - m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) - m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). - Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) - promote := m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) - - resolverFactory := projectstatusmock.NewMockResolverFactory(ctrl) + factory := projectstatusmock.NewMockResolverFactory(ctrl) resolver := projectstatusmock.NewMockResolver(ctrl) - c.projectStatusResolverFactory = resolverFactory - resolverFactory.EXPECT().For(projectstatus.ResolverConfig{QueueName: testQueue}). - After(promote). - Return(resolver, nil) - sentinel := errors.New("resolution unavailable") - resolver.EXPECT().Resolve(gomock.Any(), request, request.TerminalBuildID).Return(nil, sentinel) - - err := c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID))) - require.ErrorIs(t, err, sentinel) + c.projectStatusResolverFactory = factory + factory.EXPECT().For(projectstatus.ResolverConfig{QueueName: testQueue}).Return(resolver, nil) + resolver.EXPECT().Resolve(gomock.Any(), request, request.TerminalBuildID).Return(nil, assert.AnError) + require.ErrorIs(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID))), assert.AnError) assert.Empty(t, m.hooks.events) } @@ -416,7 +359,11 @@ func TestProcess_RecordsNamedProjectStatusResults(t *testing.T) { for _, fact := range facts { assert.Equal(t, testURI, fact.URI) assert.Equal(t, testID, fact.RequestID) - assert.Equal(t, entity.DegreeBroken, fact.Degree) + if fact.Project == "" { + assert.Equal(t, entity.DegreePartial, fact.Degree) + } else { + assert.Equal(t, entity.DegreeBroken, fact.Degree) + } assert.Positive(t, fact.CreatedAt) } require.Len(t, logs, 2) @@ -427,7 +374,7 @@ func TestProcess_RecordsNamedProjectStatusResults(t *testing.T) { func TestRecordProjectFacts_PassesEmptyTerminalBuild(t *testing.T) { ctrl := gomock.NewController(t) - c, m := newController(t, ctrl) + c, _ := newController(t, ctrl) request := requestWithState(entity.RequestStateFailed) request.TerminalBuildID = "" projectStatusResolverFactory := projectstatusmock.NewMockResolverFactory(ctrl) @@ -437,7 +384,8 @@ func TestRecordProjectFacts_PassesEmptyTerminalBuild(t *testing.T) { Return(projectStatusResolver, nil) projectStatusResolver.EXPECT().Resolve(gomock.Any(), request, "").Return(nil, nil) - err := c.recordProjectFacts(queueContext(), m.store, request) + results, err := c.resolveProjectResults(queueContext(), request) + assert.Empty(t, results) require.NoError(t, err) } @@ -454,13 +402,10 @@ func TestProcess_RejectsInvalidProjectStatusResultsBeforeWritingFacts(t *testing {Project: "project-b", Degree: math.NaN()}, }, nil) m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(requestWithState(entity.RequestStateFailed), nil) - var fact entity.ValidationFact - m.expectFactCreated(&fact) err := c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID))) require.Error(t, err) assert.ErrorContains(t, err, "invalid degree") - assert.Equal(t, wholeRepositoryProject, fact.Project) } func TestProcess_TimestampReportingFailureDoesNotFailRecord(t *testing.T) { @@ -493,8 +438,7 @@ func TestProcess_TimestampReportingFailureDoesNotFailRecord(t *testing.T) { Return(requestWithState(entity.RequestStateSucceeded), nil) var fact entity.ValidationFact m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).Return(tt.info, tt.err) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) @@ -519,8 +463,7 @@ func TestProcess_UnresolvableSourceControlCountsTimestampFailure(t *testing.T) { Return(requestWithState(entity.RequestStateSucceeded), nil) var fact entity.ValidationFact m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) require.Error(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) assert.Empty(t, m.metricsScope.Snapshot().Gauges()) @@ -573,10 +516,9 @@ func TestProcess_RecordsFactEventBeforeDerivedWork(t *testing.T) { m.queueStore.EXPECT().Get(gomock.Any(), testQueue).DoAndReturn( func(context.Context, string) (entity.Queue, error) { require.True(t, eventPersisted) - return queueRow("", "", 1), nil + return queueRow(testURI, testID, 1), nil }, ) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) @@ -722,14 +664,14 @@ func TestProcess_UnobservableDetectionLatencyDoesNotFailRecord(t *testing.T) { func TestProcess_AdoptsExistingFactFromSameRequest(t *testing.T) { tests := []struct { - name string - stored entity.ValidationFact - wantUpdate bool + name string + stored entity.ValidationFact + wantPromoteStoredFact bool }{ { - name: "green fact still advances the bookmark", - stored: entity.ValidationFact{URI: testURI, Degree: entity.DegreeGreen, RequestID: testID, CreatedAt: testChangeTime.UnixMilli()}, - wantUpdate: true, + name: "green fact still promotes the current baseline", + stored: entity.ValidationFact{URI: testURI, Degree: entity.DegreeGreen, RequestID: testID, CreatedAt: testChangeTime.UnixMilli()}, + wantPromoteStoredFact: true, }, { name: "broken fact does not", @@ -742,16 +684,15 @@ func TestProcess_AdoptsExistingFactFromSameRequest(t *testing.T) { ctrl := gomock.NewController(t) c, m := newController(t, ctrl) - // A redelivery after the fact was written but before the bookmark - // advanced: the write loses, and the stored fact decides. + // A redelivery after the fact was written but before promotion: + // the write loses, and the stored fact decides. m.reqStore.EXPECT().Get(gomock.Any(), testID). Return(requestWithState(entity.RequestStateSucceeded), nil) m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(storage.ErrAlreadyExists) m.factStore.EXPECT().Get(gomock.Any(), testURI, wholeRepositoryProject).Return(tt.stored, nil) - if tt.wantUpdate { - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + if tt.wantPromoteStoredFact { + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) @@ -776,7 +717,7 @@ func TestProcess_ExistingFactFromDifferentRequestFails(t *testing.T) { require.Error(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) } -func TestProcess_SkipsBookmarkWhenNotNewer(t *testing.T) { +func TestProcessPromotesOnlyCurrentBaselineOwner(t *testing.T) { tests := []struct { name string stored entity.Queue @@ -810,6 +751,7 @@ func TestProcess_SkipsBookmarkWhenNotNewer(t *testing.T) { // No Update: the bookmark only moves forward. if tt.wantPromote { + m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).Return(sourcecontrol.ChangeInfo{}, nil) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) } @@ -831,8 +773,7 @@ func TestProcess_SkipsPromotionWhenCommitLeftTheRef(t *testing.T) { Return(requestWithState(entity.RequestStateSucceeded), nil) var fact entity.ValidationFact m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) @@ -873,8 +814,7 @@ func TestProcess_PromotionErrorsPropagate(t *testing.T) { Return(requestWithState(entity.RequestStateSucceeded), nil) var fact entity.ValidationFact m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) tt.setup(c, m) // The bookmark already advanced, so the redelivery re-promotes the @@ -945,8 +885,7 @@ func TestProcess_AnnouncesRecordedValidation(t *testing.T) { m.expectFactCreated(&fact) if tt.wantDegree == entity.DegreeGreen { - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)).Return(nil) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) @@ -994,12 +933,7 @@ func TestProcess_AnnouncesOnlyAfterDerivedCachesMove(t *testing.T) { Return(requestWithState(entity.RequestStateSucceeded), nil) var fact entity.ValidationFact m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)). - DoAndReturn(func(context.Context, entity.Queue, int32, int32) error { - assert.Empty(t, m.hooks.events, "the bookmark must advance before the announcement") - return nil - }) + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) m.sourceControl.EXPECT().Promote(gomock.Any(), testURI). @@ -1074,33 +1008,6 @@ func TestProcess_NonTerminalRequestFails(t *testing.T) { } } -func TestProcess_RetriesBookmarkOnVersionMismatch(t *testing.T) { - ctrl := gomock.NewController(t) - c, m := newController(t, ctrl) - - stale := queueRow("", "", 1) - fresh := queueRow("", "", 2) - - m.reqStore.EXPECT().Get(gomock.Any(), testID). - Return(requestWithState(entity.RequestStateSucceeded), nil) - var fact entity.ValidationFact - m.expectFactCreated(&fact) - - gomock.InOrder( - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(stale, nil), - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), stale.Version, stale.Version+1). - Return(storage.ErrVersionMismatch), - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(fresh, nil), - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), fresh.Version, fresh.Version+1). - Return(nil), - ) - m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI). - Return(sourcecontrol.ChangeInfo{CreatedAt: testChangeTime.UnixMilli()}, nil) - m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).Return(nil) - - require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) -} - func TestProcess_MalformedRequestIDFails(t *testing.T) { ctrl := gomock.NewController(t) c, m := newController(t, ctrl) @@ -1156,18 +1063,6 @@ func TestProcess_StorageErrorsPropagate(t *testing.T) { Return(entity.Queue{}, errors.New("boom")) }, }, - { - name: "queue update fails", - setup: func(m recordMocks) { - m.reqStore.EXPECT().Get(gomock.Any(), testID). - Return(requestWithState(entity.RequestStateSucceeded), nil) - var fact entity.ValidationFact - m.expectFactCreated(&fact) - m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow("", "", 1), nil) - m.queueStore.EXPECT().Update(gomock.Any(), gomock.Any(), int32(1), int32(2)). - Return(errors.New("boom")) - }, - }, } for _, tt := range tests { @@ -1217,3 +1112,97 @@ func TestProcess_QueueMismatchFails(t *testing.T) { require.Error(t, c.Process(queueContext(), d)) } + +func TestProcessRecordsDegreeAfterResolvingProjects(t *testing.T) { + for _, tt := range []struct { + name string + state entity.RequestState + results []projectstatus.Result + degree float64 + }{ + {name: "successful repository", state: entity.RequestStateSucceeded, degree: entity.DegreeGreen}, + {name: "failed repository without project results", state: entity.RequestStateFailed, degree: entity.DegreeBroken}, + {name: "failed repository with project results", state: entity.RequestStateFailed, degree: entity.DegreePartial, + results: []projectstatus.Result{{Project: "a", Degree: entity.DegreeGreen}, {Project: "b", Degree: 0.25}}}, + {name: "successful repository with project results", state: entity.RequestStateSucceeded, degree: entity.DegreeGreen, + results: []projectstatus.Result{{Project: "a", Degree: entity.DegreeGreen}}}, + } { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + logs := requestlogmock.NewMockMaterializer(ctrl) + logs.EXPECT().PersistLog(gomock.Any(), m.store, gomock.Any()).Return(nil).AnyTimes() + c.materializer = logs + request := requestWithState(tt.state) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil) + factory := projectstatusmock.NewMockResolverFactory(ctrl) + resolver := projectstatusmock.NewMockResolver(ctrl) + c.projectStatusResolverFactory = factory + factory.EXPECT().For(projectstatus.ResolverConfig{QueueName: testQueue}).Return(resolver, nil) + resolved := false + resolver.EXPECT().Resolve(gomock.Any(), request, request.TerminalBuildID).DoAndReturn( + func(context.Context, entity.Request, string) ([]projectstatus.Result, error) { + resolved = true + return tt.results, nil + }) + facts := map[string]entity.ValidationFact{} + m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, fact entity.ValidationFact) error { + assert.True(t, resolved) + facts[fact.Project] = fact + return nil + }).Times(len(tt.results) + 1) + if tt.degree == entity.DegreeGreen { + m.queueStore.EXPECT().Get(gomock.Any(), testQueue).Return(queueRow(testURI, testID, 1), nil) + m.sourceControl.EXPECT().ChangeInfo(gomock.Any(), testURI).Return(sourcecontrol.ChangeInfo{}, nil) + m.sourceControl.EXPECT().Promote(gomock.Any(), testURI).DoAndReturn(func(context.Context, string) error { + assert.Len(t, facts, len(tt.results)+1) + assert.Empty(t, m.hooks.events) + return nil + }) + } + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) + assert.Equal(t, tt.degree, facts[""].Degree) + for _, result := range tt.results { + assert.Equal(t, result.Degree, facts[result.Project].Degree) + } + assert.Len(t, m.hooks.events, 1) + }) + } +} + +func TestProcessRetriesProjectPersistenceWithImmutablePartialFact(t *testing.T) { + ctrl := gomock.NewController(t) + c, m := newController(t, ctrl) + logs := requestlogmock.NewMockMaterializer(ctrl) + logs.EXPECT().PersistLog(gomock.Any(), m.store, gomock.Any()).Return(nil).AnyTimes() + c.materializer = logs + request := requestWithState(entity.RequestStateFailed) + m.reqStore.EXPECT().Get(gomock.Any(), testID).Return(request, nil).Times(2) + factory := projectstatusmock.NewMockResolverFactory(ctrl) + resolver := projectstatusmock.NewMockResolver(ctrl) + c.projectStatusResolverFactory = factory + factory.EXPECT().For(projectstatus.ResolverConfig{QueueName: testQueue}).Return(resolver, nil).Times(2) + resolver.EXPECT().Resolve(gomock.Any(), request, request.TerminalBuildID).Return( + []projectstatus.Result{{Project: "a", Degree: entity.DegreeBroken}}, nil).Times(2) + var stored entity.ValidationFact + m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, fact entity.ValidationFact) error { + assert.Empty(t, fact.Project) + stored = fact + return nil + }) + m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(assert.AnError) + require.ErrorIs(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID))), assert.AnError) + assert.Equal(t, entity.DegreePartial, stored.Degree) + assert.Empty(t, m.hooks.events) + + m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).Return(storage.ErrAlreadyExists) + m.factStore.EXPECT().Get(gomock.Any(), testURI, "").Return(stored, nil) + m.factStore.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, fact entity.ValidationFact) error { + assert.Equal(t, "a", fact.Project) + assert.Equal(t, entity.DegreeBroken, fact.Degree) + return nil + }) + require.NoError(t, c.Process(queueContext(), delivery(t, ctrl, recordPayload(t, testID)))) + assert.Len(t, m.hooks.events, 1) +} diff --git a/stovepipe/entity/validation_fact.go b/stovepipe/entity/validation_fact.go index 198a6d7e4..9fe9fbbfc 100644 --- a/stovepipe/entity/validation_fact.go +++ b/stovepipe/entity/validation_fact.go @@ -19,6 +19,8 @@ package entity const ( // DegreeGreen is a fully healthy scope: nothing broken. DegreeGreen = 0.0 + // DegreePartial is a broken scope with individual project results available. + DegreePartial = 0.5 // DegreeBroken is a fully broken scope. DegreeBroken = 1.0 ) @@ -41,10 +43,8 @@ type ValidationFact struct { // repository; named projects narrow the fact to part of it. Project string `json:"project"` // Degree is how broken the scope was, on the closed interval - // [DegreeGreen, DegreeBroken]. A whole-repository scope only ever takes an - // endpoint: the build either passed or it did not. Intermediate values - // belong to project-level scopes, whose analysis defines how a partial - // degree is derived. + // [DegreeGreen, DegreeBroken]. A failed repository with project results has + // DegreePartial; without project results it has DegreeBroken. Degree float64 `json:"degree"` // RequestID is the request that established the fact. RequestID string `json:"request_id"` diff --git a/stovepipe/entity/validation_fact_test.go b/stovepipe/entity/validation_fact_test.go index e5eb657f5..0341f973c 100644 --- a/stovepipe/entity/validation_fact_test.go +++ b/stovepipe/entity/validation_fact_test.go @@ -28,7 +28,8 @@ func TestValidationFact_IsGreen(t *testing.T) { }{ {name: "green degree is green", degree: DegreeGreen, expected: true}, {name: "broken degree is not green", degree: DegreeBroken, expected: false}, - {name: "partial breakage is not green", degree: 0.25, expected: false}, + {name: "partial repository is not green", degree: DegreePartial, expected: false}, + {name: "partial project breakage is not green", degree: 0.25, expected: false}, {name: "zero value is green", degree: 0, expected: true}, }