Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Do not build a job the combined result already passed for the branch

5f6163bb4051955a403b998d48f6dfabd3d6939a · Keenan Nemetz · 8h ago · parent c4ad0d5a

Do not build a job the combined result already passed for the branch

A landing lands the commit the combined result published: same tree, same
workflow, same job, already run for the base branch before it landed. The push
that moves the branch then queued all of it again, so one commit showed up twice
in the run list and the runner did the same work twice.

Queueing now asks whether a combined result for this branch already ran and passed
this job at this commit. If it did, the push owes nothing and the run that tested
the landing stays the one run for it. The question is one indexed query
(runs_repo_sha), asked only for a push to the branch itself: a tag is a release
build of its own, an event run is a new occurrence of one, and the candidate's own
run is the one being reused.

A job that deploys the site is the exception. A combined result never deploys — it
is not on the branch yet — so publishing the site is work it did not do, and that
run still happens. Repositories without a pages job, which is most of them, see the
duplicate disappear entirely.

Not in this change: a fast-forward landing still builds the head on the base branch
after the head's own branch built it. Those runs select workflows by branch, so they
are not the same work by construction; only the combined result is queued as "a
normal run for the base branch", which is what makes this dedupe exact.

Closes 01M6Z0MXZNRCEDD6HBKCJE4WCR

server/reconcile_test.go +98

@@ -7,10 +7,13 @@ import (
7 7 "log/slog"
8 8 "os"
9 9 "path/filepath"
10 "strings"
10 11 "testing"
12 "time"
11 13
12 14 "rodent.software/vole/ci/runs"
13 15 "rodent.software/vole/core/git"
16 voleid "rodent.software/vole/core/id"
14 17 "rodent.software/vole/forge/access"
15 18 "rodent.software/vole/forge/identity"
16 19 "rodent.software/vole/forge/repo"
@@ -277,3 +280,98 @@ func TestReconcileDropsTheCursorOfADeletedBranch(t *testing.T) {
277 280 t.Fatalf("the cursor for a deleted branch survived: %v", cursors)
278 281 }
279 282 }
283
284 // A combined result is queued as a run for the base branch, and the commit it builds
285 // is the commit the landing lands. The push that follows therefore owes nothing: the
286 // same tree, workflow and job already ran for this branch. Queueing them again spends
287 // the work twice and shows one commit twice in the run list (issue #249).
288 func TestALandedCombinedResultIsNotQueuedAgain(t *testing.T) {
289 t.Parallel()
290 h := newReconcileHarness(t)
291 sha := h.commit(t, "refs/heads/main", testWorkflow)
292 h.queueCombined(t, sha, "build")
293
294 h.s.reconcileRefs()
295
296 list, err := h.runs.Recent("demo", 10)
297 if err != nil {
298 t.Fatal(err)
299 }
300 if len(list) != 1 {
301 t.Fatalf("queued %d run(s) for the landed commit, want the combined result alone: %+v",
302 len(list), list)
303 }
304 if !strings.HasPrefix(list[0].Ref, runs.CandidateRefRoot) {
305 t.Fatalf("the run for the landed commit is %q, want the combined result that tested it", list[0].Ref)
306 }
307 // Skipping the run must not leave the branch owed forever: the cursor is the note
308 // that this tip was handled.
309 cursors, err := h.runs.Cursors("demo")
310 if err != nil {
311 t.Fatal(err)
312 }
313 if got := cursors["refs/heads/main"]; got != sha {
314 t.Fatalf("the branch cursor is %q, want %q", got, sha)
315 }
316 }
317
318 // A combined result never deploys the site, so a job that does is not the same work.
319 // The branch still runs it, and that run is what publishes the site for the landing.
320 func TestAJobThatDeploysTheSiteStillRunsAfterTheCombinedResult(t *testing.T) {
321 t.Parallel()
322 h := newReconcileHarness(t)
323 sha := h.commit(t, "refs/heads/main", pagesWorkflow)
324 h.queueCombined(t, sha, "build")
325
326 h.s.reconcileRefs()
327
328 list, err := h.runs.Recent("demo", 10)
329 if err != nil {
330 t.Fatal(err)
331 }
332 var branch *runs.Run
333 for _, r := range list {
334 if r.Ref == "refs/heads/main" {
335 branch = r
336 }
337 }
338 if branch == nil {
339 t.Fatalf("the site was not published by a run on the branch: %+v", list)
340 }
341 if branch.PagesPath != "public" {
342 t.Fatalf("the branch run deploys %q, want public", branch.PagesPath)
343 }
344 if len(list) != 2 {
345 t.Fatalf("queued %d run(s), want the combined result and the site run: %+v", len(list), list)
346 }
347 }
348
349 // queueCombined writes the run the combined-result reconciler would have written
350 // before the landing: the same job, for the base branch, at the commit that is now
351 // the tip.
352 func (h *reconcileHarness) queueCombined(t *testing.T, sha, job string) {
353 t.Helper()
354 id, err := voleid.New(time.Now().UTC())
355 if err != nil {
356 t.Fatal(err)
357 }
358 if _, err := h.runs.Enqueue(runs.Run{
359 ID: id, Repo: "demo", Ref: runs.CandidateRefRoot + id + "/aaaa-bbbb",
360 Branch: "main", SHA: sha, Workflow: ".vole/workflows/ci.yaml",
361 WorkflowSHA: "cc", Job: job, Status: runs.StatusSuccess,
362 Created: time.Now().UTC(),
363 }, nil); err != nil {
364 t.Fatal(err)
365 }
366 }
367
368 // pagesWorkflow declares a site, which a combined result is not allowed to deploy.
369 const pagesWorkflow = `version: 1
370 jobs:
371 build:
372 image: alpine:3.22
373 pages:
374 path: public
375 steps:
376 - run: printf 'ok\n'
377 `