diff --git a/.github/workflows/smoke-tests.yml b/.github/workflows/smoke-tests.yml index 2946b6a..d7b37e4 100644 --- a/.github/workflows/smoke-tests.yml +++ b/.github/workflows/smoke-tests.yml @@ -201,6 +201,41 @@ jobs: fi python "$RUNNER" $RUNNER_ARGS + - name: Upload the smoke report dir (per-script timings) + # PyAutoHands#264. The runner writes a consolidated smoke_timings.json + # into its report dir on every report write — one row per executed + # script/notebook with the runner's OWN measured duration, the cap in + # force, and the exit code. In the job log that data evaporates with + # the ~90-day log retention and is only recovered by hand-scraping; + # as an artifact it becomes a standing dataset the Heart board can + # ingest, for every gate run across all ten workspaces at once. + # + # WHY A GLOB and not one literal path: this workflow does not pass + # `--report-dir` — each workspace's own `.github/scripts/run_smoke.py` + # does, and the runner script is deliberately left in the workspace + # (see the header). `test-results/` is the house name, used by + # workspace-validation.yml's own run_python.py invocations; the second + # pattern catches a workspace that named its report dir something else, + # so no repo needs editing for its timings to land here. + # + # No retention-days: the dataset is the point, so it keeps the repo's + # full default artifact retention rather than a shortened window. + # + # if: always() + if-no-files-found: ignore are both load-bearing. A + # docs-only change skips the matrix entirely; a crash before the first + # report write (a failed install, a missing script list) leaves no + # report dir at all. Neither is a reason to fail a gate — and an upload + # that failed closed would turn "we collected no timings" into "the PR + # is red", which is exactly backwards. + if: always() + uses: actions/upload-artifact@v4 + with: + name: smoke-timings-${{ matrix.python-version }} + path: | + workspace/test-results/ + workspace/**/smoke_timings.json + if-no-files-found: ignore + - name: Slack notify on failure if: ${{ failure() }} uses: slackapi/slack-github-action@v1.21.0 diff --git a/tests/test_workflow_wiring.py b/tests/test_workflow_wiring.py index 180ec67..0e37a2d 100644 --- a/tests/test_workflow_wiring.py +++ b/tests/test_workflow_wiring.py @@ -71,3 +71,55 @@ def test_smoke_reusable_docs_only_gate_is_wired_fail_closed(): smoke = jobs["smoke"] assert smoke["needs"] == "changes" assert smoke["if"] == "needs.changes.outputs.docs_only != 'true'" + + +def _step(job, name_fragment): + for step in job["steps"]: + if name_fragment in step.get("name", ""): + return step + raise AssertionError(f"no step named like {name_fragment!r}") + + +def test_smoke_reusable_uploads_the_timing_dataset(): + """The per-script timing dataset leaves the job as an artifact. + + PyAutoHands#264: the runner writes smoke_timings.json into its report dir, + but a report dir dies with the runner unless something uploads it. The + artifact name carries the matrix python version so the two legs do not + collide. + """ + smoke = _load("smoke-tests.yml")["jobs"]["smoke"] + step = _step(smoke, "Upload the smoke report dir") + + assert step["uses"].startswith("actions/upload-artifact@v4") + assert step["with"]["name"] == "smoke-timings-${{ matrix.python-version }}" + assert "test-results/" in step["with"]["path"] + assert "smoke_timings.json" in step["with"]["path"] + + +def test_smoke_timings_upload_cannot_fail_the_gate(): + """A run with no timings is not a red PR. + + Both guards matter: `always()` so a failing script still yields its + timings, and `if-no-files-found: ignore` so a pre-report crash (or a + workspace whose runner wrote nothing) does not turn a missing dataset into + a failed job. + """ + smoke = _load("smoke-tests.yml")["jobs"]["smoke"] + step = _step(smoke, "Upload the smoke report dir") + + assert step["if"] == "always()" + assert step["with"]["if-no-files-found"] == "ignore" + + +def test_smoke_timings_upload_runs_before_the_slack_notifier(): + """Ordering is the reason the artifact survives a failing run. + + The Slack step is the job's terminal `failure()` hook; the upload has to + sit ahead of it so the timings for the run that just failed are collected + rather than skipped. + """ + names = [s.get("name", "") for s in _load("smoke-tests.yml")["jobs"]["smoke"]["steps"]] + 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