diff --git a/tests/code_coverage_tests/check_workflow_job_name_collisions.py b/tests/code_coverage_tests/check_workflow_job_name_collisions.py index 42e11a3e81d..849a906c3b8 100644 --- a/tests/code_coverage_tests/check_workflow_job_name_collisions.py +++ b/tests/code_coverage_tests/check_workflow_job_name_collisions.py @@ -12,13 +12,17 @@ and commit ed5761daef4ae17152446d182c860630c38b7268 carried both check runs. This invariant has to be enforced here because CI cannot enforce it on itself. A job publishes its `name:` when it sets one and its job id otherwise. Two shapes -expand that further. A `${{ matrix.key }}` reference publishes one check run per -value the matrix supplies, so two shard lists that overlap collide even though -their templates read differently. A job calling a local reusable workflow -publishes one check run per job of the callee, named ` / `, which -is why a caller's name never collides with a plain job that happens to match it. -A reference nothing resolves stays in the string, so two jobs carrying the same -unresolved template still compare equal and their collision is still caught. +expand that further. A name carrying `${{ ... }}` publishes one check run per +matrix combination it reads, so two shard lists that overlap collide even though +their templates read differently. Each expression is evaluated per combination +over the pieces a job name can hold: string literals, `matrix.`, `format()`, +`==` and `!=`, and the ` && || ` idiom, which is how the shards +reach their real ` / Run tests` names rather than staying opaque. A job +calling a local reusable workflow publishes one check run per job of the callee, +named ` / `, which is why a caller's name never collides with a +plain job that happens to match it. An expression nothing resolves stays in the +string, so two jobs carrying the same unresolved template still compare equal and +their collision is still caught. """ import itertools @@ -27,6 +31,7 @@ import re import sys from collections.abc import Iterator, Mapping, Sequence from pathlib import Path +from types import MappingProxyType from typing import Final import yaml @@ -34,7 +39,13 @@ from pydantic import BaseModel, Field, ValidationError REPO_ROOT: Final = Path(__file__).resolve().parent.parent.parent WORKFLOWS_DIR: Final = REPO_ROOT / ".github" / "workflows" -MATRIX_REF: Final = re.compile(r"\$\{\{\s*matrix\.(?P[\w-]+)\s*\}\}") +EXPRESSION: Final = re.compile(r"\$\{\{(?P.*?)\}\}", re.DOTALL) +MATRIX_KEY: Final = re.compile(r"matrix\.(?P[\w-]+)") +MATRIX_REF: Final = re.compile(r"^matrix\.(?P[\w-]+)$") +LITERAL: Final = re.compile(r"^'(?P[^']*)'$") +FORMAT_CALL: Final = re.compile(r"^format\((?P.*)\)$", re.DOTALL) +COMPARISON: Final = re.compile(r"^(?P.+?)\s*(?P==|!=)\s*(?P.+)$", re.DOTALL) +NO_MATRIX: Final[Mapping[str, str]] = MappingProxyType({}) LOCAL_CALL_PREFIX: Final = "./" @@ -97,19 +108,85 @@ def matrix_values(job: Job, key: str) -> tuple[str, ...]: return tuple(dict.fromkeys(from_list + from_rows)) -def substituted(template: str, values: Mapping[str, str]) -> str: - return MATRIX_REF.sub(lambda ref: values.get(ref.group("key"), ref.group(0)), template) +def scanned(state: tuple[int, bool], char: str) -> tuple[int, bool]: + depth, quoted = state + if char == "'": + return depth, not quoted + if quoted: + return depth, quoted + return depth + int(char == "(") - int(char == ")"), quoted + + +def split_outside(text: str, token: str) -> tuple[str, ...]: + """`text` cut on every `token` that sits outside quotes and parentheses.""" + states: Final = tuple(itertools.accumulate(text, scanned, initial=(0, False))) + cuts: Final = tuple( + index + for index in range(len(text) - len(token) + 1) + if text.startswith(token, index) and states[index] == (0, False) + ) + starts: Final = (0, *(cut + len(token) for cut in cuts)) + return tuple(text[start:end] for start, end in zip(starts, (*cuts, len(text)))) + + +def value_of(text: str, values: Mapping[str, str]) -> str | None: + expression: Final = text.strip() + literal: Final = LITERAL.match(expression) + if literal is not None: + return literal.group("text") + reference: Final = MATRIX_REF.match(expression) + if reference is not None: + return values.get(reference.group("key")) + call: Final = FORMAT_CALL.match(expression) + if call is None: + return None + arguments: Final = tuple(value_of(part, values) for part in split_outside(call.group("args"), ",")) + resolved: Final = tuple(argument for argument in arguments if argument is not None) + if not resolved or len(resolved) != len(arguments): + return None + return resolved[0].format(*resolved[1:]) + + +def holds(condition: str, values: Mapping[str, str]) -> bool | None: + comparison: Final = COMPARISON.match(condition.strip()) + if comparison is None: + return None + left: Final = value_of(comparison.group("left"), values) + right: Final = value_of(comparison.group("right"), values) + if left is None or right is None: + return None + return (left == right) == (comparison.group("operator") == "==") + + +def evaluate(body: str, values: Mapping[str, str]) -> str | None: + """The single string this expression yields, or None when its shape is not understood.""" + branches: Final = tuple(split_outside(alternative, "&&") for alternative in split_outside(body, "||")) + outcomes: Final = tuple(tuple(holds(part, values) for part in branch[:-1]) for branch in branches) + if any(outcome is None for branch in outcomes for outcome in branch): + return None + taken: Final = next((branch[-1] for branch, outcome in zip(branches, outcomes) if all(outcome)), None) + return None if taken is None else value_of(taken, values) + + +def resolved_span(span: re.Match[str], values: Mapping[str, str]) -> str: + substitution: Final = evaluate(span.group("body"), values) + return span.group(0) if substitution is None else substitution + + +def rendered(template: str, values: Mapping[str, str]) -> str: + return EXPRESSION.sub(lambda span: resolved_span(span, values), template) def expand(template: str, job: Job) -> tuple[str, ...]: - keys: Final = tuple(dict.fromkeys(ref.group("key") for ref in MATRIX_REF.finditer(template))) - candidates: Final = tuple((key, matrix_values(job, key)) for key in keys) - resolvable: Final = tuple((key, values) for key, values in candidates if values) + keys: Final = tuple(dict.fromkeys(ref.group("key") for ref in MATRIX_KEY.finditer(template))) + resolvable: Final = tuple((key, values) for key in keys if (values := matrix_values(job, key))) if not resolvable: - return (template,) + return (rendered(template, NO_MATRIX),) return tuple( - substituted(template, dict(zip((key for key, _ in resolvable), combination))) - for combination in itertools.product(*(values for _, values in resolvable)) + dict.fromkeys( + rendered(template, dict(zip((key for key, _ in resolvable), combination))) + for combination in itertools.product(*(values for _, values in resolvable)) + ) ) diff --git a/tests/code_coverage_tests/test_workflow_job_name_collisions.py b/tests/code_coverage_tests/test_workflow_job_name_collisions.py index 35c1480ecd8..96820d50040 100644 --- a/tests/code_coverage_tests/test_workflow_job_name_collisions.py +++ b/tests/code_coverage_tests/test_workflow_job_name_collisions.py @@ -1,10 +1,30 @@ -import sys -from pathlib import Path from typing import Final -sys.path.insert(0, str(Path(__file__).resolve().parent)) +from check_workflow_job_name_collisions import callee_path, collisions, parse, published, workflow_sources -from check_workflow_job_name_collisions import callee_path, collisions, parse, workflow_sources # noqa: E402 +REUSABLE_BASE: Final = """on: + workflow_call: +jobs: + run: + name: >- + ${{ matrix.python-version == '3.12' && 'Run tests' + || format('Run tests (Python {0})', matrix.python-version) }} + runs-on: ubuntu-latest + strategy: + matrix: + python-version: ["3.12", "3.13"] +""" + +SHARD_CALLER: Final = """on: pull_request +jobs: + unit: + name: ${{ matrix.shard }} + uses: ./.github/workflows/base.yml + strategy: + matrix: + include: + - shard: core-utils +""" def test_every_workflow_in_the_repo_publishes_a_unique_check_run_name() -> None: @@ -209,3 +229,38 @@ def test_a_file_that_is_not_a_workflow_is_ignored() -> None: } assert collisions(sources) == () + + +def test_a_conditional_name_expands_to_the_branch_each_matrix_value_takes() -> None: + names: Final = frozenset( + name for name, _ in published({".github/workflows/base.yml": REUSABLE_BASE, "unit.yml": SHARD_CALLER}) + ) + + assert names == frozenset({"core-utils / Run tests", "core-utils / Run tests (Python 3.13)"}) + + +def test_a_conditional_name_never_publishes_the_branch_its_condition_rules_out() -> None: + names: Final = frozenset( + name for name, _ in published({".github/workflows/base.yml": REUSABLE_BASE, "unit.yml": SHARD_CALLER}) + ) + + assert "core-utils / Run tests (Python 3.12)" not in names + + +def test_a_conditional_reusable_name_collides_with_a_plain_job_publishing_the_same_name() -> None: + sources: Final = { + ".github/workflows/base.yml": REUSABLE_BASE, + "unit.yml": SHARD_CALLER, + "postgres.yml": ("on: pull_request\njobs:\n legacy:\n name: core-utils / Run tests\n"), + } + + found: Final = collisions(sources) + + assert len(found) == 1 + assert "`core-utils / Run tests` is published by 2 jobs" in found[0] + + +def test_every_workflow_in_the_repo_resolves_every_expression_in_its_job_names() -> None: + unresolved: Final = tuple(f"{owner}: {name}" for name, owner in published(workflow_sources()) if "${{" in name) + + assert unresolved == ()