Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions .github/workflows/workspace-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,50 @@ jobs:
name: results-scripts-${{ matrix.project.name }}-${{ matrix.project.directory }}
path: workspace/test-results/
retention-days: 30
- name: Upload the per-script timings (scripts leg) under a globbable name
# PyAutoHands#264 gave RunReport.write() a consolidated
# smoke_timings.json, and PyAutoHeart#167 published it from the PR gate
# (smoke-tests.yml) as `smoke-timings-<python-version>`. This body never
# did — so the WIDEST timing sample the organism produces, the weekly
# workspace-smoke sweep, survived only INSIDE the `results-*` zips above,
# under no name a consumer could glob for.
#
# WHY A SEPARATE ARTIFACT and not a wider `results-*` glob: the artifact
# NAME is the contract the deferred Heart-board timing ingester globs on
# (`smoke-timings-*`). `results-*` is a different contract — it is what
# the `analyze` job downloads and hands to aggregate_results.py, which
# globs `**/*.json` and skips this sidecar BY NAME. Publishing timings
# under a `results-*` name would aim them straight at the one consumer
# that deliberately excludes them.
#
# WHY THE LEG SUFFIX: the PR gate has one leg per python version, so a
# fixed name is safe there. This body runs ~50 legs in a single weekly
# run, so the name must carry the leg — (project.name, project.directory)
# is exactly the pair the `results-*` upload above already relies on the
# script matrix to keep unique.
#
# WHY ONLY THE JSON, not the whole report dir: the report dir already
# ships as `results-*` above. Duplicating ~50 of them weekly buys
# nothing; the gap is discoverability, and the timings file is the part
# a consumer wants.
#
# NO retention-days (unlike the 30 above, deliberately): the dataset IS
# the point, matching the PR gate's reasoning — the timings keep the
# repo's full default artifact retention and outlive the report dirs
# they came from. One small JSON per leg, so the cost is negligible.
#
# if: always() + if-no-files-found: ignore are both load-bearing. A leg
# that fails still has timings worth keeping; a leg that dies before the
# first report write (a failed install, an empty script list) leaves no
# report dir at all — and an upload failing closed would turn "we
# collected no timings" into "the weekly sweep is red", which is exactly
# backwards.
if: always()
uses: actions/upload-artifact@v4
with:
name: smoke-timings-scripts-${{ matrix.project.name }}-${{ matrix.project.directory }}
path: workspace/**/smoke_timings.json
if-no-files-found: ignore

run_notebooks:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -453,6 +497,21 @@ jobs:
name: results-notebooks-${{ matrix.project.name }}-${{ matrix.project.directory }}
path: workspace/test-results/
retention-days: 30
- name: Upload the per-script timings (notebooks leg) under a globbable name
# The notebook leg's report is written by run.py, which reaches the same
# RunReport.write() — so it emits smoke_timings.json too, and it is half
# the weekly timing sample. Full rationale on the scripts leg's copy of
# this step above; only the name's leg segment and the gate differ.
#
# The gate is carried over from the results-* upload: the *_test
# workspaces publish no notebooks, so those legs skip execution entirely
# and have nothing to collect.
if: always() && steps.gate.outputs.run == 'true'
uses: actions/upload-artifact@v4
with:
name: smoke-timings-notebooks-${{ matrix.project.name }}-${{ matrix.project.directory }}
path: workspace/**/smoke_timings.json
if-no-files-found: ignore

analyze:
runs-on: ubuntu-latest
Expand Down
85 changes: 85 additions & 0 deletions tests/test_workflow_wiring.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,88 @@ def test_smoke_timings_upload_runs_before_the_slack_notifier():
upload = next(i for i, n in enumerate(names) if "Upload the smoke report dir" in n)
slack = next(i for i, n in enumerate(names) if "Slack notify" in n)
assert upload < slack


# --- the weekly sweep's timings need a name too (PyAutoHeart, follow-on to #167) ---
#
# smoke-tests.yml is the PR gate. The WEEKLY sweep runs through
# workspace-validation.yml, which had no named timing upload at all — its
# smoke_timings.json survived only inside the per-leg `results-*` zips, under no
# name a consumer could glob for. These tests pin the fix in both directions:
# the timings are published as `smoke-timings-*`, and they stay OUT of the
# `results-*` namespace the aggregate consumer reads.

TIMING_LEGS = (("run_scripts", "scripts"), ("run_notebooks", "notebooks"))


def _timing_step(job_name):
jobs = _load("workspace-validation.yml")["jobs"]
return _step(jobs[job_name], "Upload the per-script timings")


def test_validation_body_publishes_the_timing_dataset():
"""Both weekly legs upload smoke_timings.json under a `smoke-timings-*` name."""
for job_name, _leg in TIMING_LEGS:
step = _timing_step(job_name)
assert step["uses"].startswith("actions/upload-artifact@v4")
assert step["with"]["name"].startswith("smoke-timings-")
assert "smoke_timings.json" in step["with"]["path"]


def test_validation_timing_artifact_names_carry_the_leg():
"""A fixed name is safe on the PR gate and fatal here.

The gate has one leg per python version; this body runs ~50 legs in one
weekly run, so the name has to carry the (project, directory) pair the
script matrix keeps unique — exactly as the sibling `results-*` upload does.
"""
for job_name, leg in TIMING_LEGS:
name = _timing_step(job_name)["with"]["name"]
assert name == (
f"smoke-timings-{leg}-"
"${{ matrix.project.name }}-${{ matrix.project.directory }}"
)


def test_validation_timing_upload_cannot_fail_the_weekly_sweep():
"""No timings is not a red sweep — same reasoning as the PR gate's copy."""
for job_name, _leg in TIMING_LEGS:
step = _timing_step(job_name)
assert step["if"].startswith("always()")
assert step["with"]["if-no-files-found"] == "ignore"


def test_validation_notebook_timing_upload_keeps_the_no_notebooks_gate():
"""The *_test workspaces publish no notebooks — that leg never executes."""
assert _timing_step("run_notebooks")["if"] == (
"always() && steps.gate.outputs.run == 'true'"
)


def test_validation_timing_dataset_keeps_full_default_retention():
"""The dataset is the point.

The sibling `results-*` uploads expire at 30 days; the timings deliberately
carry no retention-days so they keep the repo's full default window and
outlive the report dirs they were extracted from (the PR gate's upload
omits it for the same reason).
"""
for job_name, _leg in TIMING_LEGS:
assert "retention-days" not in _timing_step(job_name)["with"]


def test_timing_artifacts_stay_out_of_the_aggregate_namespace():
"""`results-*` and `smoke-timings-*` are two different contracts.

`analyze` downloads `results-*` and hands it to aggregate_results.py, which
globs `**/*.json` and skips the timing sidecar BY NAME. Naming the timing
artifacts into that pattern would aim them at the one consumer that
deliberately excludes them.
"""
analyze = _load("workspace-validation.yml")["jobs"]["analyze"]
pattern = _step(analyze, "Download all result artifacts")["with"]["pattern"]
assert pattern == "results-*"

prefix = pattern.rstrip("*")
for job_name, _leg in TIMING_LEGS:
assert not _timing_step(job_name)["with"]["name"].startswith(prefix)
Loading