Skip to content

Commit ee9833d

Browse files
authored
Merge pull request #1350 from DeusData/fix/skip-tests-skips-smoke
fix(release): skip_tests silently skipped smoke and soak too
2 parents c16da50 + 1670414 commit ee9833d

2 files changed

Lines changed: 112 additions & 2 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,23 @@ jobs:
8080
attest: true
8181

8282
# ── 4. Smoke test every binary ──────────────────────────────────
83+
# !cancelled() && !failure(): GitHub propagates "skipped" TRANSITIVELY down the
84+
# needs graph, so with skip_tests=true a skipped `test` skipped smoke and soak
85+
# too — even though `build` overrode the same condition and succeeded. Nothing
86+
# failed and nothing said so; the pipeline simply carried on toward publishing
87+
# artifacts that had never been smoke-tested or soaked. Every job downstream of
88+
# an optional phase needs this override, or the phase being optional silently
89+
# makes the phases after it optional as well.
8390
smoke:
91+
if: ${{ !cancelled() && !failure() }}
8492
needs: [build]
8593
uses: ./.github/workflows/_smoke.yml
8694
with:
8795
broad_platforms: true
8896

8997
# ── 5. Soak tests ──────────────────────────────────────────────
9098
soak:
91-
if: ${{ inputs.soak_level != 'none' }}
99+
if: ${{ !cancelled() && !failure() && inputs.soak_level != 'none' }}
92100
needs: [build]
93101
uses: ./.github/workflows/_soak.yml
94102
with:
@@ -97,9 +105,17 @@ jobs:
97105
version: ${{ inputs.version }}
98106

99107
# ── 6. Create DRAFT release ────────────────────────────────────
108+
# Requires smoke to have actually SUCCEEDED, not merely "not failed". The bare
109+
# !cancelled() && !failure() form is fail-OPEN: a skipped smoke is neither
110+
# cancelled nor failed, so a draft was one condition away from being cut from
111+
# binaries nobody had run. soak may legitimately be 'skipped' (soak_level=none),
112+
# so that one is enumerated explicitly rather than lumped in.
100113
release-draft:
101114
needs: [smoke, soak]
102-
if: ${{ !cancelled() && !failure() }}
115+
if: >-
116+
${{ !cancelled() && !failure()
117+
&& needs.smoke.result == 'success'
118+
&& (needs.soak.result == 'success' || needs.soak.result == 'skipped') }}
103119
runs-on: ubuntu-latest
104120
permissions:
105121
contents: write
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
#!/usr/bin/env bash
2+
# Contract: making a release phase optional must not silently make the phases
3+
# AFTER it optional too, and a draft must require smoke to have actually run.
4+
#
5+
# This exists because it nearly shipped untested binaries. `skip_tests: true`
6+
# skipped `test`, and GitHub propagates "skipped" TRANSITIVELY down the needs
7+
# graph: `build` overrode the condition and ran, but `smoke` and `soak` had no
8+
# override and were skipped. Nothing failed. Nothing said so. `release-draft`
9+
# then ran anyway, because `!cancelled() && !failure()` is fail-OPEN — a skipped
10+
# job is neither cancelled nor failed — so the pipeline was one gate away from
11+
# publishing artifacts nobody had smoke-tested or soaked.
12+
#
13+
# Two properties are pinned:
14+
# 1. every job downstream of an optional phase carries the
15+
# `!cancelled() && !failure()` override, so it runs when an ancestor was
16+
# deliberately skipped;
17+
# 2. release-draft requires `needs.smoke.result == 'success'` explicitly, so a
18+
# skipped smoke BLOCKS the draft instead of sailing past it.
19+
set -euo pipefail
20+
21+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
22+
WF="$ROOT/.github/workflows/release.yml"
23+
[ -f "$WF" ] || { echo "FAIL: $WF not found" >&2; exit 2; }
24+
25+
python3 - "$WF" <<'PY'
26+
import pathlib
27+
import re
28+
import sys
29+
30+
text = pathlib.Path(sys.argv[1]).read_text()
31+
32+
# Slice the file into top-level job blocks: two-space indented "name:".
33+
blocks, current, name = {}, [], None
34+
for line in text.splitlines():
35+
m = re.match(r"^ ([A-Za-z0-9_-]+):\s*$", line)
36+
if m:
37+
if name:
38+
blocks[name] = "\n".join(current)
39+
name, current = m.group(1), []
40+
continue
41+
if name is not None:
42+
current.append(line)
43+
if name:
44+
blocks[name] = "\n".join(current)
45+
46+
failures = []
47+
48+
def cond(job):
49+
"""The job's `if:` expression, block scalars folded onto one line."""
50+
body = blocks.get(job, "")
51+
m = re.search(r"^ if:\s*(>-|>|\|-|\|)?\s*(.*?)(?=^ [a-z_-]+:|\Z)",
52+
body, re.S | re.M)
53+
if not m:
54+
return ""
55+
return " ".join(m.group(2).split())
56+
57+
# 1. Downstream-of-optional jobs must tolerate a deliberately skipped ancestor.
58+
# `test` is the optional phase (if: !inputs.skip_tests); everything after it
59+
# in the chain has to survive that.
60+
TOLERATE = ["build", "smoke", "soak", "release-draft"]
61+
for job in TOLERATE:
62+
if job not in blocks:
63+
failures.append(f"{job}: job missing from release.yml — update this contract")
64+
continue
65+
c = cond(job)
66+
if "!cancelled()" not in c or "!failure()" not in c:
67+
failures.append(
68+
f"{job}: `if:` lacks `!cancelled() && !failure()` (got: {c or '<none>'}).\n"
69+
f" With skip_tests=true a skipped ancestor SKIPS this job silently.")
70+
71+
# 2. The draft must require smoke to have genuinely succeeded.
72+
draft = cond("release-draft")
73+
if "needs.smoke.result == 'success'" not in draft:
74+
failures.append(
75+
"release-draft: `if:` must require needs.smoke.result == 'success'.\n"
76+
" `!cancelled() && !failure()` alone is fail-open: a SKIPPED smoke\n"
77+
" passes it, and the draft gets cut from unsmoked binaries.")
78+
79+
# soak is allowed to be skipped (soak_level=none) but must be enumerated, not
80+
# ignored — otherwise 'none' and 'silently never ran' are indistinguishable.
81+
if "needs.soak.result" not in draft:
82+
failures.append(
83+
"release-draft: `if:` must mention needs.soak.result explicitly so a\n"
84+
" legitimately skipped soak (soak_level=none) is distinguishable\n"
85+
" from a soak that never ran.")
86+
87+
if failures:
88+
for f in failures:
89+
print("FAIL: " + f, file=sys.stderr)
90+
print(f"release gate-chain contract FAILED with {len(failures)} violation(s)",
91+
file=sys.stderr)
92+
sys.exit(1)
93+
print("PASS: optional phases cannot silently disable the phases after them")
94+
PY

0 commit comments

Comments
 (0)