Skip to content

Commit 0b6525a

Browse files
committed
chore(ci): address fixture release review feedback
1 parent b2ac847 commit 0b6525a

10 files changed

Lines changed: 389 additions & 166 deletions

File tree

.github/actions/build-evm-base/action.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ outputs:
2525
evm-bin:
2626
description: "Binary name of the evm tool to use"
2727
value: ${{ steps.config-evm-reader.outputs.evm-bin }}
28-
x-dist:
28+
xdist:
2929
description: "Number of parallel pytest-xdist workers to use"
30-
value: ${{ steps.config-evm-reader.outputs.x-dist }}
30+
value: ${{ steps.config-evm-reader.outputs.xdist }}
3131
runs:
3232
using: "composite"
3333
steps:
@@ -56,7 +56,7 @@ runs:
5656
echo "Repository: ${{ steps.resolved.outputs.repo }}"
5757
echo "Reference: ${{ steps.resolved.outputs.ref }}"
5858
echo "EVM Binary: ${{ steps.config-evm-reader.outputs.evm-bin }}"
59-
echo "X-Dist parameter: ${{ steps.config-evm-reader.outputs.x-dist }}"
59+
echo "X-Dist parameter: ${{ steps.config-evm-reader.outputs.xdist }}"
6060
- name: Skip building for EELS
6161
if: steps.config-evm-reader.outputs.impl == 'eels'
6262
shell: bash

.github/actions/build-fixtures/action.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ runs:
6464
# Allow exit code 5 (NO_TESTS_COLLECTED) for fork ranges with no tests.
6565
EXIT_CODE=0
6666
if [ "${{ steps.evm-builder.outputs.impl }}" = "eels" ]; then
67-
uv run fill -n ${{ steps.evm-builder.outputs.x-dist }} ${{ steps.properties.outputs.fill-params }} $FORK_ARGS $OUTPUT_ARG --build-name ${{ inputs.release_name }} --no-html --durations=100 --log-level=DEBUG || EXIT_CODE=$?
67+
uv run fill -n ${{ steps.evm-builder.outputs.xdist }} ${{ steps.properties.outputs.fill-params }} $FORK_ARGS $OUTPUT_ARG --build-name ${{ inputs.release_name }} --no-html --durations=100 --log-level=DEBUG || EXIT_CODE=$?
6868
else
69-
uv run fill -n ${{ steps.evm-builder.outputs.x-dist }} --evm-bin=${{ steps.evm-builder.outputs.evm-bin }} ${{ steps.properties.outputs.fill-params }} $FORK_ARGS $OUTPUT_ARG --build-name ${{ inputs.release_name }} --no-html --durations=100 --log-level=DEBUG || EXIT_CODE=$?
69+
uv run fill -n ${{ steps.evm-builder.outputs.xdist }} --evm-bin=${{ steps.evm-builder.outputs.evm-bin }} ${{ steps.properties.outputs.fill-params }} $FORK_ARGS $OUTPUT_ARG --build-name ${{ inputs.release_name }} --no-html --durations=100 --log-level=DEBUG || EXIT_CODE=$?
7070
fi
7171
if [ "$EXIT_CODE" -ne 0 ] && [ "$EXIT_CODE" -ne 5 ]; then
7272
exit "$EXIT_CODE"

.github/configs/evm.yaml

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,33 @@
1-
# Client aliases
1+
# `benchmark` is a client alias (resolves to geth) referenced by name in
2+
# .github/workflows/benchmark.yaml.
23
benchmark:
34
impl: geth
45
repo: ethereum/go-ethereum
56
ref: master
67
evm-bin: evm
7-
x-dist: auto
8-
consensus:
9-
impl: eels
10-
repo: null
11-
ref: null
12-
evm-bin: null
13-
x-dist: auto
14-
8+
xdist: auto
159
eels:
1610
impl: eels
1711
repo: null
1812
ref: null
1913
evm-bin: null
20-
x-dist: auto
14+
xdist: auto
2115
evmone:
2216
impl: evmone
2317
repo: ethereum/evmone
2418
ref: master
2519
targets: ["evmone-t8n"]
2620
evm-bin: evmone-t8n
27-
x-dist: auto
21+
xdist: auto
2822
geth:
2923
impl: geth
3024
repo: ethereum/go-ethereum
3125
ref: master
3226
evm-bin: evm
33-
x-dist: auto
27+
xdist: auto
3428
besu:
3529
impl: besu
3630
repo: hyperledger/besu
3731
ref: main
3832
evm-bin: evmtool
39-
x-dist: 0
33+
xdist: 0

.github/configs/feature.yaml

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,29 @@
1+
# Release feature definitions consumed by the release_fixtures workflow.
2+
#
3+
# Each top-level key is either:
4+
# 1. A literal feature name used in the release tag, e.g. `benchmark` ->
5+
# tag `tests-benchmark@vX.Y.Z`. The default `tests` feature tags as
6+
# `tests@vX.Y.Z` directly (not `tests-tests@`).
7+
# 2. The shared `devnet` entry, which is resolved by `-devnet` suffix:
8+
# any workflow input feature ending in `-devnet` (e.g. `bal-devnet`,
9+
# `glamsterdam-devnet`) maps here while keeping its `<feat>-devnet`
10+
# name in the tag (`tests-<feat>-devnet@vX.Y.Z`). The devnet index
11+
# is encoded in the version (X), not in the feature name -- so
12+
# `glamsterdam-devnet-5` (branch) releases as feature
13+
# `glamsterdam-devnet` at version `v5.0.0`, yielding the tag
14+
# `tests-glamsterdam-devnet@v5.0.0`. This means `feature.yaml` does
15+
# not need to be updated for new devnets.
16+
#
117
# Unless filling for special features, all features should fill for previous forks (starting from Frontier) too
2-
consensus:
3-
evm-type: consensus
4-
fill-params: --until=BPO4
18+
tests:
19+
evm-type: eels
20+
fill-params: --until=BPO4 --generate-all-formats
521

622
benchmark:
723
evm-type: benchmark
824
fill-params: --fork=Amsterdam --gas-benchmark-values 1,10,30,60,100,150 ./tests/benchmark
925

26+
# Shared entry for all `<feat>-devnet` releases; matched by `-devnet` suffix.
1027
devnet:
1128
evm-type: eels
12-
fill-params: --until=Amsterdam
29+
fill-params: --until=Amsterdam --generate-all-formats

.github/scripts/generate_build_matrix.py

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@
2121
import re
2222
import sys
2323
from pathlib import Path
24+
from typing import NoReturn
2425

2526
import yaml
2627

2728
FEATURE_CONFIG = Path(".github/configs/feature.yaml")
2829
FORK_RANGES_CONFIG = Path(".github/configs/fork-ranges.yaml")
2930

31+
VERSION_RE = re.compile(r"^v[0-9]+\.[0-9]+\.[0-9]+$")
32+
3033
# Canonical fork ordering used to filter fork ranges per feature.
3134
FORK_ORDER = [
3235
"Frontier",
@@ -64,6 +67,65 @@ def load_config(path: Path) -> dict:
6467
return yaml.safe_load(f)
6568

6669

70+
def fail(message: str) -> NoReturn:
71+
"""Print an error to stderr and exit non-zero."""
72+
print(f"Error: {message}", file=sys.stderr)
73+
sys.exit(1)
74+
75+
76+
def validate_inputs(feature: str, version: str, branch: str) -> None:
77+
"""
78+
Validate the release dispatch inputs before building a matrix.
79+
80+
Centralize the feature/version checks here so they are unit-testable
81+
rather than living as inline bash in the release workflow.
82+
83+
For ``<feat>-devnet`` releases the major version (``X`` of ``vX.Y.Z``)
84+
must equal the devnet number encoded in the release branch, so a
85+
``bal-devnet`` release from ``bal-devnet-7`` must be tagged ``v7.*.*``.
86+
"""
87+
if not feature:
88+
fail("feature name is empty")
89+
if not VERSION_RE.match(version):
90+
fail(f"version '{version}' must match vX.Y.Z (e.g. v20.0.0)")
91+
92+
# A bare `devnet` has no friendly `<feat>-` prefix to tag with.
93+
if feature in ("devnet", "-devnet"):
94+
fail("devnet releases require a <feat>- prefix, e.g. bal-devnet")
95+
96+
# `<feat>-devnet-<n>`: the devnet index belongs in the version (X of
97+
# vX.Y.Z), not in the feature name.
98+
if "-devnet-" in feature:
99+
suggested_feature, _, suggested_index = feature.rpartition("-")
100+
fail(
101+
"devnet index must go in 'version', not the feature name; "
102+
f"did you mean feature={suggested_feature} "
103+
f"version=v{suggested_index}.0.0?"
104+
)
105+
106+
if feature.endswith("-devnet"):
107+
if not branch:
108+
fail(
109+
"devnet releases require a 'branch' input, "
110+
"e.g. branch=bal-devnet-7"
111+
)
112+
match = re.search(r"(\d+)$", branch)
113+
if not match:
114+
fail(
115+
f"could not parse a devnet number from branch '{branch}' "
116+
"(expected a trailing number, e.g. bal-devnet-7)"
117+
)
118+
devnet_number = int(match.group(1))
119+
major = int(version.lstrip("v").split(".")[0])
120+
if major != devnet_number:
121+
minor_patch = version.split(".", 1)[1]
122+
fail(
123+
f"version major (v{major}) must equal the devnet number "
124+
f"({devnet_number}) from branch '{branch}'; "
125+
f"did you mean version=v{devnet_number}.{minor_patch}?"
126+
)
127+
128+
67129
def parse_until_fork(fill_params: str) -> str | None:
68130
"""
69131
Extract the ``--until`` value from fill-params.
@@ -134,16 +196,22 @@ def build_matrix(
134196

135197
def main() -> None:
136198
"""Entry point."""
137-
if len(sys.argv) != 2:
199+
args = sys.argv[1:]
200+
if len(args) < 2:
138201
print(
139-
"Usage: generate_build_matrix.py <feature>",
202+
"Usage: generate_build_matrix.py <feature> <version> [branch]",
140203
file=sys.stderr,
141204
)
142205
sys.exit(1)
143206

207+
name = args[0]
208+
version = args[1]
209+
branch = args[2] if len(args) > 2 else ""
210+
211+
validate_inputs(name, version, branch)
212+
144213
config = load_config(FEATURE_CONFIG)
145214
fork_ranges = load_config(FORK_RANGES_CONFIG) or []
146-
name = sys.argv[1]
147215

148216
# `<feat>-devnet` releases (e.g. bal-devnet) share the `devnet` entry,
149217
# while keeping their friendly name in the matrix and artifact outputs.

.github/scripts/tests/test_release_scripts.py

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@ class TestGenerateBuildMatrix:
4343

4444
def test_split_feature_produces_entries_per_range(self):
4545
"""Verify a split feature expands into one entry per range."""
46-
result = run_script(BUILD_MATRIX_SCRIPT, "consensus")
46+
result = run_script(BUILD_MATRIX_SCRIPT, "tests", "v24.0.0")
4747
assert result.returncode == 0
4848
out = parse_matrix_output(result.stdout)
4949
matrix = json.loads(out["build_matrix"])
5050
assert len(matrix) > 1
51-
assert out["feature_name"] == "consensus"
51+
assert out["feature_name"] == "tests"
5252
assert out["combine_labels"] != ""
5353
labels = [e["label"] for e in matrix]
5454
assert all(lbl != "" for lbl in labels)
@@ -57,7 +57,7 @@ def test_split_feature_produces_entries_per_range(self):
5757

5858
def test_unsplit_feature_produces_single_entry(self):
5959
"""Verify a feature without fork-ranges produces one entry."""
60-
result = run_script(BUILD_MATRIX_SCRIPT, "benchmark")
60+
result = run_script(BUILD_MATRIX_SCRIPT, "benchmark", "v24.0.0")
6161
assert result.returncode == 0
6262
out = parse_matrix_output(result.stdout)
6363
matrix = json.loads(out["build_matrix"])
@@ -70,7 +70,9 @@ def test_unsplit_feature_produces_single_entry(self):
7070

7171
def test_devnet_name_resolves_to_shared_feature(self):
7272
"""Verify a <feat>-devnet name resolves to the devnet feature."""
73-
result = run_script(BUILD_MATRIX_SCRIPT, "bal-devnet")
73+
result = run_script(
74+
BUILD_MATRIX_SCRIPT, "bal-devnet", "v7.0.0", "bal-devnet-7"
75+
)
7476
assert result.returncode == 0
7577
out = parse_matrix_output(result.stdout)
7678
matrix = json.loads(out["build_matrix"])
@@ -80,7 +82,7 @@ def test_devnet_name_resolves_to_shared_feature(self):
8082

8183
def test_unknown_feature_fails(self):
8284
"""Verify error exit for unknown feature name."""
83-
result = run_script(BUILD_MATRIX_SCRIPT, "nonexistent")
85+
result = run_script(BUILD_MATRIX_SCRIPT, "nonexistent", "v1.0.0")
8486
assert result.returncode == 1
8587
assert "not found" in result.stderr
8688

@@ -92,7 +94,7 @@ def test_no_args_fails(self):
9294

9395
def test_output_is_valid_github_actions_format(self):
9496
"""Verify output lines are key=value for GITHUB_OUTPUT."""
95-
result = run_script(BUILD_MATRIX_SCRIPT, "consensus")
97+
result = run_script(BUILD_MATRIX_SCRIPT, "tests", "v24.0.0")
9698
assert result.returncode == 0
9799
lines = result.stdout.strip().splitlines()
98100
assert len(lines) == 3
@@ -101,6 +103,57 @@ def test_output_is_valid_github_actions_format(self):
101103
assert lines[2].startswith("combine_labels=")
102104

103105

106+
class TestValidateInputs:
107+
"""Test the release input validation in generate_build_matrix.py."""
108+
109+
def test_bad_version_fails(self):
110+
"""Verify a non vX.Y.Z version is rejected."""
111+
result = run_script(BUILD_MATRIX_SCRIPT, "tests", "24.0.0")
112+
assert result.returncode == 1
113+
assert "must match vX.Y.Z" in result.stderr
114+
115+
def test_bare_devnet_fails(self):
116+
"""Verify a bare `devnet` feature name is rejected."""
117+
result = run_script(
118+
BUILD_MATRIX_SCRIPT, "devnet", "v7.0.0", "bal-devnet-7"
119+
)
120+
assert result.returncode == 1
121+
assert "require a <feat>- prefix" in result.stderr
122+
123+
def test_devnet_index_in_feature_name_fails(self):
124+
"""Verify `<feat>-devnet-<n>` is rejected with a suggestion."""
125+
result = run_script(
126+
BUILD_MATRIX_SCRIPT, "bal-devnet-7", "v7.0.0", "bal-devnet-7"
127+
)
128+
assert result.returncode == 1
129+
assert "did you mean feature=bal-devnet version=v7.0.0" in (
130+
result.stderr
131+
)
132+
133+
def test_devnet_without_branch_fails(self):
134+
"""Verify a `<feat>-devnet` release requires a branch."""
135+
result = run_script(BUILD_MATRIX_SCRIPT, "bal-devnet", "v7.0.0")
136+
assert result.returncode == 1
137+
assert "require a 'branch' input" in result.stderr
138+
139+
def test_devnet_major_must_match_branch_number(self):
140+
"""Verify the major version must equal the branch devnet number."""
141+
result = run_script(
142+
BUILD_MATRIX_SCRIPT, "bal-devnet", "v3.0.0", "bal-devnet-7"
143+
)
144+
assert result.returncode == 1
145+
assert "must equal the devnet number" in result.stderr
146+
147+
def test_devnet_matching_major_passes(self):
148+
"""Verify a matching major/branch number builds the matrix."""
149+
result = run_script(
150+
BUILD_MATRIX_SCRIPT, "bal-devnet", "v7.0.0", "bal-devnet-7"
151+
)
152+
assert result.returncode == 0
153+
out = parse_matrix_output(result.stdout)
154+
assert out["feature_name"] == "bal-devnet"
155+
156+
104157
class TestCreateReleaseTarball:
105158
"""Test create_release_tarball.py."""
106159

0 commit comments

Comments
 (0)