From 33ac03af644da34a9bebf754e5c701f289ad0e9c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:10:38 +0000 Subject: [PATCH 1/5] test(e2e): assert workflow depends_on enforces ordering Without this test, a regression in the queue's dep-tracking logic could silently allow a downstream workflow to start before its dependency finishes. Final status would still be 'success', making the bug invisible to existing scenario fixtures. The test uses step timestamps (Started/Finished) to prove the dependent workflow did not begin until the dependency's step had fully completed, matching the failure mode reported in #3858 where a docker image built by the upstream workflow was missing when the downstream tried to pull it. Verified: removing depends_on from the fixture causes an assertion failure with the exact timestamp inversion described in #3858. Refs #3858 --- e2e/scenarios/depends_on_ordering_test.go | 119 ++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 e2e/scenarios/depends_on_ordering_test.go diff --git a/e2e/scenarios/depends_on_ordering_test.go b/e2e/scenarios/depends_on_ordering_test.go new file mode 100644 index 00000000000..aa1ca2bffe9 --- /dev/null +++ b/e2e/scenarios/depends_on_ordering_test.go @@ -0,0 +1,119 @@ +// Copyright 2026 Woodpecker Authors +// +// 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. + +//go:build test + +package scenarios + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.woodpecker-ci.org/woodpecker/v3/e2e/setup" + forge_types "go.woodpecker-ci.org/woodpecker/v3/server/forge/types" + "go.woodpecker-ci.org/woodpecker/v3/server/model" + "go.woodpecker-ci.org/woodpecker/v3/server/pipeline" +) + +// Models woodpecker-ci/woodpecker#3858: "depends_on seems to be broken on +// workflow level". A downstream workflow that depends_on an upstream one +// started running before the upstream had finished — in the reporter's case +// the upstream built a docker image (auth-build:${CI_COMMIT_SHA}) that the +// downstream then tried to use, getting "pull access denied ... repository +// does not exist" because the build had not completed yet. +// +// build-auth sleeps for a measurable duration. auth-server-tests depends_on it. +// Correct behaviour: auth-server-tests must not START until build-auth has +// FINISHED. We prove this directly from the recorded step timestamps rather +// than just final status, because a broken ordering still ends "success" — +// the steps just overlap in time. + +var buildAuthYAML = []byte(` +skip_clone: true + +steps: + - name: build-auth + image: dummy + environment: + SLEEP: '2s' + commands: + - echo building auth-build image +`) + +var authServerTestsYAML = []byte(` +skip_clone: true + +depends_on: + - build-auth + +steps: + - name: auth-server-tests + image: dummy + commands: + - echo running tests against built image +`) + +// TestWorkflowDependsOnOrdering asserts that a workflow with a workflow-level +// depends_on does not begin executing until its dependency has completed. +func TestWorkflowDependsOnOrdering(t *testing.T) { + env := setup.StartServer(t.Context(), t, []*forge_types.FileMeta{ + {Name: ".woodpecker/build-auth.yaml", Data: buildAuthYAML}, + {Name: ".woodpecker/auth-server-tests.yaml", Data: authServerTestsYAML}, + }) + agent := setup.StartAgent(t, env.GRPCAddr) + setup.WaitForAgentRegistered(t, env.Store, agent) + + created, err := pipeline.Create(t.Context(), env.Store, env.Fixtures.Repo, &model.Pipeline{ + Event: model.EventPush, + Branch: "main", + Commit: "deadbeef", + Ref: "refs/heads/main", + Author: env.Fixtures.Owner.Login, + Sender: env.Fixtures.Owner.Login, + }) + require.NoError(t, err, "create pipeline") + require.NotNil(t, created) + + finished := setup.WaitForPipeline(t, env.Store, created.ID) + require.Equal(t, model.StatusSuccess, finished.Status, "pipeline should succeed") + + // Both workflows should have succeeded. + workflows, err := env.Store.WorkflowGetTree(finished) + require.NoError(t, err, "list workflows") + byWorkflow := make(map[string]*model.Workflow, len(workflows)) + for _, w := range workflows { + byWorkflow[w.Name] = w + } + require.Contains(t, byWorkflow, "build-auth", "build-auth workflow present") + require.Contains(t, byWorkflow, "auth-server-tests", "auth-server-tests workflow present") + assert.Equal(t, model.StatusSuccess, byWorkflow["build-auth"].State) + assert.Equal(t, model.StatusSuccess, byWorkflow["auth-server-tests"].State) + + // The core assertion: the dependent step must start only AFTER the + // dependency step finished. Compare recorded timestamps. + buildStep := setup.WaitForStep(t, env.Store, finished, "build-auth") + testStep := setup.WaitForStep(t, env.Store, finished, "auth-server-tests") + + require.NotZero(t, buildStep.Finished, "build-auth must record a finish time") + require.NotZero(t, testStep.Started, "auth-server-tests must record a start time") + + // This is the line that fails if #3858 regresses: a broken workflow-level + // depends_on lets auth-server-tests start while build-auth is still + // sleeping, so testStep.Started < buildStep.Finished. + assert.GreaterOrEqualf(t, testStep.Started, buildStep.Finished, + "auth-server-tests started at %d but build-auth only finished at %d — dependent workflow ran before its dependency completed (issue #3858)", + testStep.Started, buildStep.Finished) +} From c3e54843515ce0602a5c24c7e25faa1a811c2678 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:17:58 +0000 Subject: [PATCH 2/5] test(e2e): use spaced workflow names matching issue #3858 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closer reproduction: the reporter used filenames with spaces ('Build Auth.yml', 'Auth server tests.yml') and depended on the workflow name derived from those filenames. Using the same names exercises the exact path — including name resolution through SanitizePath — that was broken in the original report. Refs #3858 --- e2e/scenarios/depends_on_ordering_test.go | 39 +++++++++++++---------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/e2e/scenarios/depends_on_ordering_test.go b/e2e/scenarios/depends_on_ordering_test.go index aa1ca2bffe9..39798553500 100644 --- a/e2e/scenarios/depends_on_ordering_test.go +++ b/e2e/scenarios/depends_on_ordering_test.go @@ -35,17 +35,19 @@ import ( // downstream then tried to use, getting "pull access denied ... repository // does not exist" because the build had not completed yet. // -// build-auth sleeps for a measurable duration. auth-server-tests depends_on it. -// Correct behaviour: auth-server-tests must not START until build-auth has +// "Build Auth" sleeps for a measurable duration. "Auth server tests" depends_on it. +// Correct behaviour: "Auth server tests" must not START until "Build Auth" has // FINISHED. We prove this directly from the recorded step timestamps rather // than just final status, because a broken ordering still ends "success" — // the steps just overlap in time. +// Workflow and step names match the issue report verbatim so the test +// reads as a direct reproduction of the failure scenario. var buildAuthYAML = []byte(` skip_clone: true steps: - - name: build-auth + - name: Build Auth image: dummy environment: SLEEP: '2s' @@ -57,10 +59,10 @@ var authServerTestsYAML = []byte(` skip_clone: true depends_on: - - build-auth + - Build Auth steps: - - name: auth-server-tests + - name: Auth server tests image: dummy commands: - echo running tests against built image @@ -70,8 +72,11 @@ steps: // depends_on does not begin executing until its dependency has completed. func TestWorkflowDependsOnOrdering(t *testing.T) { env := setup.StartServer(t.Context(), t, []*forge_types.FileMeta{ - {Name: ".woodpecker/build-auth.yaml", Data: buildAuthYAML}, - {Name: ".woodpecker/auth-server-tests.yaml", Data: authServerTestsYAML}, + // Filenames with spaces: the workflow name is derived from the + // filename (minus extension), so "Build Auth.yaml" → workflow "Build Auth", + // matching exactly what the issue reporter used. + {Name: ".woodpecker/Build Auth.yaml", Data: buildAuthYAML}, + {Name: ".woodpecker/Auth server tests.yaml", Data: authServerTestsYAML}, }) agent := setup.StartAgent(t, env.GRPCAddr) setup.WaitForAgentRegistered(t, env.Store, agent) @@ -97,23 +102,23 @@ func TestWorkflowDependsOnOrdering(t *testing.T) { for _, w := range workflows { byWorkflow[w.Name] = w } - require.Contains(t, byWorkflow, "build-auth", "build-auth workflow present") - require.Contains(t, byWorkflow, "auth-server-tests", "auth-server-tests workflow present") - assert.Equal(t, model.StatusSuccess, byWorkflow["build-auth"].State) - assert.Equal(t, model.StatusSuccess, byWorkflow["auth-server-tests"].State) + require.Contains(t, byWorkflow, "Build Auth", "Build Auth workflow present") + require.Contains(t, byWorkflow, "Auth server tests", "Auth server tests workflow present") + assert.Equal(t, model.StatusSuccess, byWorkflow["Build Auth"].State) + assert.Equal(t, model.StatusSuccess, byWorkflow["Auth server tests"].State) // The core assertion: the dependent step must start only AFTER the // dependency step finished. Compare recorded timestamps. - buildStep := setup.WaitForStep(t, env.Store, finished, "build-auth") - testStep := setup.WaitForStep(t, env.Store, finished, "auth-server-tests") + buildStep := setup.WaitForStep(t, env.Store, finished, "Build Auth") + testStep := setup.WaitForStep(t, env.Store, finished, "Auth server tests") - require.NotZero(t, buildStep.Finished, "build-auth must record a finish time") - require.NotZero(t, testStep.Started, "auth-server-tests must record a start time") + require.NotZero(t, buildStep.Finished, "Build Auth must record a finish time") + require.NotZero(t, testStep.Started, "Auth server tests must record a start time") // This is the line that fails if #3858 regresses: a broken workflow-level - // depends_on lets auth-server-tests start while build-auth is still + // depends_on lets Auth server tests start while Build Auth is still // sleeping, so testStep.Started < buildStep.Finished. assert.GreaterOrEqualf(t, testStep.Started, buildStep.Finished, - "auth-server-tests started at %d but build-auth only finished at %d — dependent workflow ran before its dependency completed (issue #3858)", + "Auth server tests started at %d but Build Auth only finished at %d — dependent workflow ran before its dependency completed (issue #3858)", testStep.Started, buildStep.Finished) } From c2a2c57ca1862a47afdee5725df6d4a5bdee0ff1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:22:16 +0000 Subject: [PATCH 3/5] test(e2e): add multi-workflow DAG ordering test for #3858 The single-edge test only proves one depends_on link. The issue example was a chain (Build base -> Build Auth -> Auth server tests) and the author also described a fan-in flow where a workflow depends on two parents. Add a 4-workflow DAG covering: - a transitive chain (3 levels deep) - two consumers of the same dependency running in parallel - a fan-in where the final workflow must wait for the slowest of its two parents Each dependency edge is asserted independently via step timestamps, so a regression on any single edge is pinpointed by name. Refs #3858 --- e2e/scenarios/depends_on_ordering_test.go | 142 ++++++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/e2e/scenarios/depends_on_ordering_test.go b/e2e/scenarios/depends_on_ordering_test.go index 39798553500..f1aafdd70ae 100644 --- a/e2e/scenarios/depends_on_ordering_test.go +++ b/e2e/scenarios/depends_on_ordering_test.go @@ -122,3 +122,145 @@ func TestWorkflowDependsOnOrdering(t *testing.T) { "Auth server tests started at %d but Build Auth only finished at %d — dependent workflow ran before its dependency completed (issue #3858)", testStep.Started, buildStep.Finished) } + +// The full chain from the issue body: +// +// Build base (no deps, runs first) +// └─ Build Auth (depends_on: [Build base]) +// └─ Auth server tests (depends_on: [Build Auth]) +// +// Plus a fan-in edge case: "Lint" also depends only on "Build base" and runs +// in parallel with "Build Auth", and "Auth server tests" additionally depends +// on "Lint" — so it must wait for the slowest of its two dependencies. +// +// Final DAG: +// +// Build base ─┬─> Build Auth ─┐ +// └─> Lint ────────┴─> Auth server tests + +var chainBuildBaseYAML = []byte(` +skip_clone: true + +steps: + - name: Build base + image: dummy + environment: + SLEEP: '2s' + commands: + - echo building base image +`) + +var chainBuildAuthYAML = []byte(` +skip_clone: true + +depends_on: + - Build base + +steps: + - name: Build Auth + image: dummy + environment: + SLEEP: '2s' + commands: + - echo building auth-build image +`) + +var chainLintYAML = []byte(` +skip_clone: true + +depends_on: + - Build base + +steps: + - name: Lint + image: dummy + environment: + SLEEP: '3s' + commands: + - echo linting +`) + +var chainAuthServerTestsYAML = []byte(` +skip_clone: true + +depends_on: + - Build Auth + - Lint + +steps: + - name: Auth server tests + image: dummy + commands: + - echo running tests against built image +`) + +// TestWorkflowDependsOnChainOrdering reproduces the multi-stage DAG from the +// issue report and asserts every dependency edge is respected, including a +// fan-in where the final workflow must wait for the slowest of two parents. +func TestWorkflowDependsOnChainOrdering(t *testing.T) { + env := setup.StartServer(t.Context(), t, []*forge_types.FileMeta{ + {Name: ".woodpecker/Build base.yaml", Data: chainBuildBaseYAML}, + {Name: ".woodpecker/Build Auth.yaml", Data: chainBuildAuthYAML}, + {Name: ".woodpecker/Lint.yaml", Data: chainLintYAML}, + {Name: ".woodpecker/Auth server tests.yaml", Data: chainAuthServerTestsYAML}, + }) + agent := setup.StartAgent(t, env.GRPCAddr) + setup.WaitForAgentRegistered(t, env.Store, agent) + + created, err := pipeline.Create(t.Context(), env.Store, env.Fixtures.Repo, &model.Pipeline{ + Event: model.EventPush, + Branch: "main", + Commit: "deadbeef", + Ref: "refs/heads/main", + Author: env.Fixtures.Owner.Login, + Sender: env.Fixtures.Owner.Login, + }) + require.NoError(t, err, "create pipeline") + require.NotNil(t, created) + + finished := setup.WaitForPipeline(t, env.Store, created.ID) + require.Equal(t, model.StatusSuccess, finished.Status, "pipeline should succeed") + + // All four workflows should have succeeded. + workflows, err := env.Store.WorkflowGetTree(finished) + require.NoError(t, err, "list workflows") + byWorkflow := make(map[string]*model.Workflow, len(workflows)) + for _, w := range workflows { + byWorkflow[w.Name] = w + } + for _, name := range []string{"Build base", "Build Auth", "Lint", "Auth server tests"} { + require.Containsf(t, byWorkflow, name, "%s workflow present", name) + assert.Equalf(t, model.StatusSuccess, byWorkflow[name].State, "%s should succeed", name) + } + + base := setup.WaitForStep(t, env.Store, finished, "Build base") + build := setup.WaitForStep(t, env.Store, finished, "Build Auth") + lint := setup.WaitForStep(t, env.Store, finished, "Lint") + tests := setup.WaitForStep(t, env.Store, finished, "Auth server tests") + + for name, step := range map[string]*model.Step{ + "Build base": base, "Build Auth": build, "Lint": lint, "Auth server tests": tests, + } { + require.NotZerof(t, step.Started, "%s must record a start time", name) + require.NotZerof(t, step.Finished, "%s must record a finish time", name) + } + + // Edge 1: Build Auth waits for Build base. + assertStartsAfter(t, "Build Auth", build.Started, "Build base", base.Finished) + // Edge 2: Lint waits for Build base (the second consumer of the same dep). + assertStartsAfter(t, "Lint", lint.Started, "Build base", base.Finished) + // Edge 3: Auth server tests waits for Build Auth. + assertStartsAfter(t, "Auth server tests", tests.Started, "Build Auth", build.Finished) + // Edge 4 (fan-in): Auth server tests waits for Lint too — and since Lint is + // the slower parent, this is the binding constraint. + assertStartsAfter(t, "Auth server tests", tests.Started, "Lint", lint.Finished) +} + +// assertStartsAfter fails if dependent started before dependency finished, +// with a message naming both workflows and the offending timestamps. +func assertStartsAfter(t *testing.T, dependent string, dependentStarted int64, dependency string, dependencyFinished int64) { + t.Helper() + assert.GreaterOrEqualf(t, dependentStarted, dependencyFinished, + "%q started at %d but its dependency %q only finished at %d — dependent workflow ran before its dependency completed (issue #3858)", + dependent, dependentStarted, dependency, dependencyFinished) +} From 06fac01db0e9f35e45202156d1396e28dc538f94 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:25:11 +0000 Subject: [PATCH 4/5] test(e2e): shorten depends_on ordering sleeps Step timestamps are unix-second granularity, so a 1s base sleep and 2s slow-parent sleep are enough to land an out-of-order start in a strictly earlier second while keeping detection reliable. Cuts the two tests from ~7.3s to ~4.3s. Refs #3858 --- e2e/scenarios/depends_on_ordering_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/scenarios/depends_on_ordering_test.go b/e2e/scenarios/depends_on_ordering_test.go index f1aafdd70ae..c0b42904adc 100644 --- a/e2e/scenarios/depends_on_ordering_test.go +++ b/e2e/scenarios/depends_on_ordering_test.go @@ -50,7 +50,7 @@ steps: - name: Build Auth image: dummy environment: - SLEEP: '2s' + SLEEP: '1s' commands: - echo building auth-build image `) @@ -145,7 +145,7 @@ steps: - name: Build base image: dummy environment: - SLEEP: '2s' + SLEEP: '1s' commands: - echo building base image `) @@ -160,7 +160,7 @@ steps: - name: Build Auth image: dummy environment: - SLEEP: '2s' + SLEEP: '1s' commands: - echo building auth-build image `) @@ -175,7 +175,7 @@ steps: - name: Lint image: dummy environment: - SLEEP: '3s' + SLEEP: '2s' commands: - echo linting `) From 7a615eaeee5b402725c5cc44b52ce55073de1e62 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 9 Jun 2026 11:38:07 +0000 Subject: [PATCH 5/5] test(e2e): fix lint in depends_on ordering test godot wanted the comment sentence punctuated correctly and misspell (US locale) rejected 'behaviour'. Reword the doc comment to drop the embedded ellipsis that tripped godot and use US spelling. Refs #3858 --- e2e/scenarios/depends_on_ordering_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/e2e/scenarios/depends_on_ordering_test.go b/e2e/scenarios/depends_on_ordering_test.go index c0b42904adc..54907a6dee2 100644 --- a/e2e/scenarios/depends_on_ordering_test.go +++ b/e2e/scenarios/depends_on_ordering_test.go @@ -30,13 +30,13 @@ import ( // Models woodpecker-ci/woodpecker#3858: "depends_on seems to be broken on // workflow level". A downstream workflow that depends_on an upstream one -// started running before the upstream had finished — in the reporter's case +// started running before the upstream had finished. In the reporter's case // the upstream built a docker image (auth-build:${CI_COMMIT_SHA}) that the -// downstream then tried to use, getting "pull access denied ... repository -// does not exist" because the build had not completed yet. +// downstream then tried to use, getting a "pull access denied" error because +// the build had not completed yet. // // "Build Auth" sleeps for a measurable duration. "Auth server tests" depends_on it. -// Correct behaviour: "Auth server tests" must not START until "Build Auth" has +// Correct behavior: "Auth server tests" must not START until "Build Auth" has // FINISHED. We prove this directly from the recorded step timestamps rather // than just final status, because a broken ordering still ends "success" — // the steps just overlap in time.