From 5829bbbc7f5f500f53f1260383cc7a22fc1113e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 21:35:02 +0000 Subject: [PATCH] ci: publish the weekly sweep's smoke timings under a globbable name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `RunReport.write()` emits a consolidated `smoke_timings.json` into every report dir (PyAutoHands d2a22f4), and PyAutoHeart#167 published it from the PR gate as `smoke-timings-`. The weekly sweep runs through a different body — workspace-validation.yml — which got no such upload, so the widest timing sample the organism produces survived only inside the per-leg `results-*` zips, under no name a consumer could glob for, and expired with them at 30 days. Confirmed on weekly run 32688412626: 109 artifacts, not one `smoke-timings-*`. Mirror the gate's named upload into the `run_scripts` and `run_notebooks` legs. The name carries the leg — `smoke-timings---` — because the gate has two legs per run and this body has ~50; that is the same (project, directory) pair the sibling `results-*` names already rely on to stay unique. Only the JSON is uploaded (the report dir already ships as `results-*`) and deliberately with no `retention-days`, so the dataset keeps the repo's full default window and outlives the zips it came from. The new artifacts stay out of the `results-*` namespace on purpose: that is what `analyze` downloads and hands to `aggregate_results.py`, which globs `**/*.json` and skips this sidecar by name. smoke-tests.yml is untouched — no change to PR-gate behaviour. Six tests in test_workflow_wiring.py pin the names, the two fail-open guards, the notebook leg's no-notebooks gate, the absent retention-days, and the namespace separation. Full suite: 641 passed. Closes #181 --- .github/workflows/workspace-validation.yml | 59 +++++++++++++++ tests/test_workflow_wiring.py | 85 ++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/.github/workflows/workspace-validation.yml b/.github/workflows/workspace-validation.yml index 8319976..7c4d9f3 100644 --- a/.github/workflows/workspace-validation.yml +++ b/.github/workflows/workspace-validation.yml @@ -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-`. 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 @@ -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 diff --git a/tests/test_workflow_wiring.py b/tests/test_workflow_wiring.py index 0e37a2d..744e1d7 100644 --- a/tests/test_workflow_wiring.py +++ b/tests/test_workflow_wiring.py @@ -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)