Run the e2e suite in CI, and make it say what it actually did - #123
Conversation
One `e2e` job, per pull request, outside the python matrix: plain unnested `pytest -m e2e` on the runner's own Docker. A runner is an ephemeral VM with Docker preinstalled, so there is no host to protect and no nested daemon to build; devpod is already in the lockfile setup-pixi restores. The job is only worth having if its result means something, and today it would not. `skipped` names two unrelated outcomes in this suite -- thirteen tests declining an opt-in, and a test that could not reach a registry -- so a run that built nothing at all printed the same summary line as a healthy one. This gives the deliberate case a word of its own, treats every other skip as the failure it is, and has the session refuse to exit zero having created no workspaces. Closes #114
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideAdds a dedicated per-PR e2e CI job that runs the real devpod/Docker-based suite, and introduces an e2e guard mechanism so runs accurately report what they did by treating undeclared skips as failures and enforcing a minimum level of actual workspace creation. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #123 +/- ##
=======================================
Coverage 92.76% 92.76%
=======================================
Files 18 18
Lines 2005 2005
=======================================
Hits 1860 1860
Misses 145 145 🚀 New features to boost your workflow:
|
`test_git_status_via_ssh` handed devpod the fixture's *bare* repo -- the stand-in for a remote its siblings use as a clone source. devpod opens a local path as the workspace folder rather than cloning it, so the container came up sitting in `objects/`, `refs/` and a `core.bare=true` config, where `git status` exits 128 saying it must be run in a work tree. That is not environment-specific; it could never have been anything else. It had simply never been observed. Until the creation step was made unskippable, `devpod up` without `--ide none` exited 1 on any headless machine and both assertions sat behind an `if result.returncode == 0` that was never true -- a test that passed by not running. The first session to reach them was the first run of this suite in CI. The source is now the working copy, which is what a test about git in a container needed all along, and the rc assertion carries the output: devpod reports a failed remote command as its own exit 1 and buries git's message in its logging.
c5aeb38 to
586abe1
Compare
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Fixed point 85e27b1...586abe1. All checks green, CI / e2e present on both pushes. I re-read the four cited runs from the logs rather than reproducing anything locally.
The sabotage evidence holds, exactly as claimed. Run 31256036009 log line 331: 4 passed, 18 skipped, 992 deselected in 2.77s, zero failures — and line 332 ##[error]Process completed with exit code 1, with the shortfall printed at line 311. Run 31256477982 line 355: a bare pytest.skip reported as FAILED test/e2e/test_full_workflow.py::...::test_dl_version_command, against the test's own name. Run 31255686455 line 1779 shows Process exited with status 128 from git status — consistent with a bare repo and nothing else. Final run 37s (12:08:09→12:08:46) against py310–py313 at 42/42/51/52s. Every number in the body checks out.
I also confirmed the vacuous case the ledger's tests_attempted == 0 carve-out leaves open is closed by pytest itself: with everything deselected pytest exits 5, so a job that selects no e2e tests is red regardless. That carve-out is safe.
Standards
S1 — the ledger's central invariant is false (blocking). create_e2e_workspace is documented as "the only door", but it takes an injectable run=, and test/unit/test_e2e_workspace_helper.py:57,70,102 calls it with a stub — each stubbed success appends to the module-global LEDGER. Nothing in pixi run test / test-e2e / ci reaches it (addopts and -m e2e keep the two sets disjoint), so no shipped path is broken today. But "no report of a passing test can answer whether a container was built" is precisely the claim, and a pytest -m "" run makes three phantom workspaces clear the floor. Record only when run is subprocess.run, or reset the ledger in pytest_sessionstart.
S2 — the opt-out protocol is a magic substring, and it did not have to be. is_declared_opt_out matches "declared e2e opt-out: " inside str(report.longrepr). e2e_guard.py:24-27 justifies this with "the report a hook sees at the far end of the run is text". That is not correct: pytest_runtest_makereport(item, call) receives the live call.excinfo, and the hook here explicitly discards it (# noqa: ARG001). A class DeclaredOptOut(Skipped) checked via call.excinfo.errisinstance(...) is unforgeable, and fixes two live consequences of the string form:
report.skippedis True for xfail. Any@pytest.mark.xfailadded undertest/e2e/is silently rewritten into a hard failure. No xfail exists there today; the trap is loaded.-raattributes the skip to the helper, not the test: run 31256546714 line 313 readsSKIPPED [1] test/fixtures/e2e_guard.py:39. In a PR about output that names things accurately, the opt-out line does not name who opted out.
S3 — the floor's correction does not land in the summary line, contrary to the design principle the module states twice ("the correction has to land in the summary line"). The F-rewrite half does. The floor half does not: the last line of the sabotage run is still 4 passed, 18 skipped in green, with the shortfall 20 rows above it. That is the byte-identical line #114 complained about. Mitigated by the exit code, but the stated principle is only half delivered — worth either honest wording or a terminalreporter.stats entry so the count moves.
S4 — speculative generality (minor). shortfalls() -> List[str] is only ever length 0 or 1, and pytest_sessionfinish ignores its contents. devpod_available() now has exactly one caller — require_devpod(), in the same file.
S5 — stale dependency floor (minor). pytest>=7.4 in [project.optional-dependencies], but @pytest.hookimpl(wrapper=True) is pytest-8/pluggy-1.2 syntax. The lock (pytest 9.0.2) and frozen: true protect CI; the declared range is unsound.
S6 — the README teaches a number no run prints (blocking, one line). README:439 shows 9 e2e tests attempted, 4 workspaces created. Every real run prints 22. This is in the paragraph telling the reader that this is the line to read when a green tick is in doubt.
Spec
Against #114 and its comment.
- "one
e2ejob in.github/workflows/ci.yml, outside the py310–py313 matrix" — met, verified:e2eis a sibling job, one run per push, one image pull. - "say which you chose and why, and make the choice easy to reverse" — met. The reversal instruction is slightly wrong, though: with
push: branches: [main]still inon:, addingschedule:plusif: github.event_name != 'pull_request'yields everything except PRs, not nightly. One extra clause, or amend the comment. - "Verify the job triggers on a pull request and that a deliberately broken e2e test turns it red — do not accept a green tick from a job that skipped everything" — met and exceeded; the second sabotage covers the case a broken test cannot.
- Comment: "assert a floor on what actually ran… Distinguishing 'opted out by design' from 'could not run' is the real requirement; today they are the same word" — met.
4 passed, 18 skippednow exits 1. - Gap 1 (blocking, prose). README:426 and the CHANGELOG both say the suite runs "on every pull request".
on.pull_request.branches: [main]means a PR onto any non-main base — a stacked PR, which this repo's tooling produces — runs no CI at all, e2e included. Either widen the trigger or stop claiming "every". - Gap 2 (follow-up, outside the diff). The
mainruleset requires exactly one check:ci (py313).e2eis advisory — a red e2e job does not block a merge. #114 asks that "a job that silently does not run is worse than one that fails"; a job whose failure nothing enforces is the same family. Not fixable in a diff, and #114 did not ask for it, so it is not a change request — but it should be a ticket, or this PR's tick still does not gate anything.
The test_git_status_via_ssh fix — correct. git_fixtures.py:165 returns remote_url as the bare remote_dir; a bare repo has no work tree and git status exits 128 there, which is exactly what run 31255686455 line 1779 shows. Pointing at work_dir (which carries .devcontainer/devcontainer.json, added at git_fixtures.py:182) is right, and the test now genuinely exercises git-in-a-container. The added rc message is a real improvement over assert 1 == 0.
Ruling on the mounts-not-clones residual — deferring is correct. Three tests pass remote_url to devpod up; devpod mounts it, so none exercises a clone and remote_url is a remote to nothing. But those three assert only that a workspace exists, and they do that truthfully — so the suite this PR wires into CI is narrower than its names suggest, not hollow. Widening it is a different ticket with a different shape (it needs a real remote, or an accepted local-clone path), and folding it in here would have delayed a guard whose absence is the actual live hazard. One thing worth adding to that ticket, which I could not settle from the logs: since the bare directory contains no .devcontainer/ on disk, those three workspaces are probably built from devpod's fallback image rather than the fixture's declared one — meaning git_fixtures.py's devcontainer.json may be exercised by exactly one test in the suite. Unverified.
Does opt_out make the distinction unrepresentable? No — it makes it loud, which is nearly as good, with one hole. A new test writing bare pytest.skip is caught and reported as FAILED (proven on the runner), so the wrong thing is not silent. But it remains writable, and one route escapes entirely: pytest.skip(..., allow_module_level=True) raises during collection, produces a CollectReport, and never reaches pytest_runtest_makereport. A module-level skip of test_full_workflow.py would still be caught by the floor (no workspaces); a module-level skip of test_interactive_session.py would vanish 13 tests green. The two mechanisms cover each other partially, not completely. See S2 for the stronger encoding.
CHANGELOG entry. Voice and structure match 0.0.14–0.0.16 well — explanatory prose in the bullets, mechanism before consequence. Two corrections:
- The Fixed bullet reverses its own meaning. "
test_git_status_via_sshtests git in a container again, having been pointed at the fixture's bare repository — which has no work tree" reads as though pointing it at the bare repo is the fix. It was the bug. Needs "having previously been pointed at" or a recast. - No lead paragraph. 0.0.14, 0.0.15 and 0.0.16 each open with a framing sentence naming user impact ("Nothing about how you install or run
dlchanges"). This entry is entirely developer-facing and should say so before the sections.
Accuracy otherwise checks out: devpod is in [tool.pixi.dependencies] as claimed, "five per-test guards" is exactly the five devpod_available() checks removed, and "it finishes before the matrix does" is true on the one measurement taken.
Verdict
Request changes — small ones. Nothing about the mechanism is wrong, and the evidence for it is the strongest I have reviewed on this map; every claim in the body reproduced from the logs.
Blocking (all cheap):
- S1 —
create_e2e_workspace's injectedrun=lets stubbed unit calls credit the globalLEDGERwith workspaces that do not exist, falsifying the guard's central invariant. - S6 — README:439 prints
9 e2e tests attempted; every run prints22. - Spec Gap 1 — README:426 and the CHANGELOG claim "every pull request"; the trigger is
branches: [main]only. - CHANGELOG #1 — the dangling modifier that reverses the git-status bullet.
Follow-up tickets, not blocking: S2 (exception-typed opt-out; xfail trap; skip attributed to the helper), S3 (floor's correction never reaches the summary line), Spec Gap 2 (e2e is not a required check on main), the mounts-not-clones widening, and the module-level-skip escape. S4/S5 are notes.
timeout-minutes: 25 is the right call: ~40x the observed 37s, and the failure it guards (640 B/s on 1.25 GB ≈ 22 days) blows any bound, so the number only has to be small enough to return a red tick the same day. Nothing in the workflow can make the job silently not run on a PR to main — no if:, no continue-on-error, and an all-deselected run exits 5.
blooop
left a comment
blooop
left a comment
There was a problem hiding this comment.
This was generated by AI during review.
Fixed point 85e27b1...586abe1. All checks green, CI / e2e present on both pushes. I re-read the four cited runs from the logs rather than reproducing anything locally.
The sabotage evidence holds, exactly as claimed. Run 31256036009 log line 331: 4 passed, 18 skipped, 992 deselected in 2.77s, zero failures — and line 332 ##[error]Process completed with exit code 1, with the shortfall printed at line 311. Run 31256477982 line 355: a bare pytest.skip reported as FAILED test/e2e/test_full_workflow.py::...::test_dl_version_command, against the test's own name. Run 31255686455 line 1779 shows Process exited with status 128 from git status — consistent with a bare repo and nothing else. Final run 37s (12:08:09→12:08:46) against py310–py313 at 42/42/51/52s. Every number in the body checks out.
I also confirmed the vacuous case the ledger's tests_attempted == 0 carve-out leaves open is closed by pytest itself: with everything deselected pytest exits 5, so a job that selects no e2e tests is red regardless. That carve-out is safe.
Standards
S1 — the ledger's central invariant is false (blocking). create_e2e_workspace is documented as "the only door", but it takes an injectable run=, and test/unit/test_e2e_workspace_helper.py:57,70,102 calls it with a stub — each stubbed success appends to the module-global LEDGER. Nothing in pixi run test / test-e2e / ci reaches it (addopts and -m e2e keep the two sets disjoint), so no shipped path is broken today. But "no report of a passing test can answer whether a container was built" is precisely the claim, and a pytest -m "" run makes three phantom workspaces clear the floor. Record only when run is subprocess.run, or reset the ledger in pytest_sessionstart.
S2 — the opt-out protocol is a magic substring, and it did not have to be. is_declared_opt_out matches "declared e2e opt-out: " inside str(report.longrepr). e2e_guard.py:24-27 justifies this with "the report a hook sees at the far end of the run is text". That is not correct: pytest_runtest_makereport(item, call) receives the live call.excinfo, and the hook here explicitly discards it (# noqa: ARG001). A class DeclaredOptOut(Skipped) checked via call.excinfo.errisinstance(...) is unforgeable, and fixes two live consequences of the string form:
report.skippedis True for xfail. Any@pytest.mark.xfailadded undertest/e2e/is silently rewritten into a hard failure. No xfail exists there today; the trap is loaded.-raattributes the skip to the helper, not the test: run 31256546714 line 313 readsSKIPPED [1] test/fixtures/e2e_guard.py:39. In a PR about output that names things accurately, the opt-out line does not name who opted out.
S3 — the floor's correction does not land in the summary line, contrary to the design principle the module states twice ("the correction has to land in the summary line"). The F-rewrite half does. The floor half does not: the last line of the sabotage run is still 4 passed, 18 skipped in green, with the shortfall 20 rows above it. That is the byte-identical line #114 complained about. Mitigated by the exit code, but the stated principle is only half delivered — worth either honest wording or a terminalreporter.stats entry so the count moves.
S4 — speculative generality (minor). shortfalls() -> List[str] is only ever length 0 or 1, and pytest_sessionfinish ignores its contents. devpod_available() now has exactly one caller — require_devpod(), in the same file.
S5 — stale dependency floor (minor). pytest>=7.4 in [project.optional-dependencies], but @pytest.hookimpl(wrapper=True) is pytest-8/pluggy-1.2 syntax. The lock (pytest 9.0.2) and frozen: true protect CI; the declared range is unsound.
S6 — the README teaches a number no run prints (blocking, one line). README:439 shows 9 e2e tests attempted, 4 workspaces created. Every real run prints 22. This is in the paragraph telling the reader that this is the line to read when a green tick is in doubt.
Spec
Against #114 and its comment.
- "one
e2ejob in.github/workflows/ci.yml, outside the py310–py313 matrix" — met, verified:e2eis a sibling job, one run per push, one image pull. - "say which you chose and why, and make the choice easy to reverse" — met. The reversal instruction is slightly wrong, though: with
push: branches: [main]still inon:, addingschedule:plusif: github.event_name != 'pull_request'yields everything except PRs, not nightly. One extra clause, or amend the comment. - "Verify the job triggers on a pull request and that a deliberately broken e2e test turns it red — do not accept a green tick from a job that skipped everything" — met and exceeded; the second sabotage covers the case a broken test cannot.
- Comment: "assert a floor on what actually ran… Distinguishing 'opted out by design' from 'could not run' is the real requirement; today they are the same word" — met.
4 passed, 18 skippednow exits 1. - Gap 1 (blocking, prose). README:426 and the CHANGELOG both say the suite runs "on every pull request".
on.pull_request.branches: [main]means a PR onto any non-main base — a stacked PR, which this repo's tooling produces — runs no CI at all, e2e included. Either widen the trigger or stop claiming "every". - Gap 2 (follow-up, outside the diff). The
mainruleset requires exactly one check:ci (py313).e2eis advisory — a red e2e job does not block a merge. #114 asks that "a job that silently does not run is worse than one that fails"; a job whose failure nothing enforces is the same family. Not fixable in a diff, and #114 did not ask for it, so it is not a change request — but it should be a ticket, or this PR's tick still does not gate anything.
The test_git_status_via_ssh fix — correct. git_fixtures.py:165 returns remote_url as the bare remote_dir; a bare repo has no work tree and git status exits 128 there, which is exactly what run 31255686455 line 1779 shows. Pointing at work_dir (which carries .devcontainer/devcontainer.json, added at git_fixtures.py:182) is right, and the test now genuinely exercises git-in-a-container. The added rc message is a real improvement over assert 1 == 0.
Ruling on the mounts-not-clones residual — deferring is correct. Three tests pass remote_url to devpod up; devpod mounts it, so none exercises a clone and remote_url is a remote to nothing. But those three assert only that a workspace exists, and they do that truthfully — so the suite this PR wires into CI is narrower than its names suggest, not hollow. Widening it is a different ticket with a different shape (it needs a real remote, or an accepted local-clone path), and folding it in here would have delayed a guard whose absence is the actual live hazard. One thing worth adding to that ticket, which I could not settle from the logs: since the bare directory contains no .devcontainer/ on disk, those three workspaces are probably built from devpod's fallback image rather than the fixture's declared one — meaning git_fixtures.py's devcontainer.json may be exercised by exactly one test in the suite. Unverified.
Does opt_out make the distinction unrepresentable? No — it makes it loud, which is nearly as good, with one hole. A new test writing bare pytest.skip is caught and reported as FAILED (proven on the runner), so the wrong thing is not silent. But it remains writable, and one route escapes entirely: pytest.skip(..., allow_module_level=True) raises during collection, produces a CollectReport, and never reaches pytest_runtest_makereport. A module-level skip of test_full_workflow.py would still be caught by the floor (no workspaces); a module-level skip of test_interactive_session.py would vanish 13 tests green. The two mechanisms cover each other partially, not completely. See S2 for the stronger encoding.
CHANGELOG entry. Voice and structure match 0.0.14–0.0.16 well — explanatory prose in the bullets, mechanism before consequence. Two corrections:
- The Fixed bullet reverses its own meaning. "
test_git_status_via_sshtests git in a container again, having been pointed at the fixture's bare repository — which has no work tree" reads as though pointing it at the bare repo is the fix. It was the bug. Needs "having previously been pointed at" or a recast. - No lead paragraph. 0.0.14, 0.0.15 and 0.0.16 each open with a framing sentence naming user impact ("Nothing about how you install or run
dlchanges"). This entry is entirely developer-facing and should say so before the sections.
Accuracy otherwise checks out: devpod is in [tool.pixi.dependencies] as claimed, "five per-test guards" is exactly the five devpod_available() checks removed, and "it finishes before the matrix does" is true on the one measurement taken.
Verdict
Request changes — small ones. Nothing about the mechanism is wrong, and the evidence for it is the strongest I have reviewed on this map; every claim in the body reproduced from the logs.
Blocking (all cheap):
- S1 —
create_e2e_workspace's injectedrun=lets stubbed unit calls credit the globalLEDGERwith workspaces that do not exist, falsifying the guard's central invariant. - S6 — README:439 prints
9 e2e tests attempted; every run prints22. - Spec Gap 1 — README:426 and the CHANGELOG claim "every pull request"; the trigger is
branches: [main]only. - CHANGELOG #1 — the dangling modifier that reverses the git-status bullet.
Follow-up tickets, not blocking: S2 (exception-typed opt-out; xfail trap; skip attributed to the helper), S3 (floor's correction never reaches the summary line), Spec Gap 2 (e2e is not a required check on main), the mounts-not-clones widening, and the module-level-skip escape. S4/S5 are notes.
timeout-minutes: 25 is the right call: ~40x the observed 37s, and the failure it guards (640 B/s on 1.25 GB ≈ 22 days) blows any bound, so the number only has to be small enough to return a red tick the same day. Nothing in the workflow can make the job silently not run on a PR to main — no if:, no continue-on-error, and an all-deselected run exits 5.
The guard said three things that were not quite so. **The ledger's only door had a second one.** `create_e2e_workspace` takes an injectable `run=` so its own logic can be unit-tested without devpod, and each of those stubbed calls appended a workspace to the session ledger. No shipped path reached it -- `test` and `test-e2e` select disjoint sets -- but `pytest -m ""` credited three containers that never existed, and three is enough to clear the floor. The ledger is now credited only when the real runner was the thing that ran, which a stub cannot be. **The opt-out was a magic substring, and it did not have to be.** Skips were told apart by grepping the report text, justified by "the report a hook sees at the far end of the run is text". `pytest_runtest_makereport` has `call.excinfo` -- the live exception -- and the hook was throwing it away. Opting out now raises a `DeclaredOptOut`, and the hook asks the exception what it is. That closes a trap that was loaded and not yet sprung: `report.skipped` is also true for xfail, so any `@pytest.mark.xfail` added under `test/e2e/` would have been rewritten into a hard failure. It also puts the `-ra` line against the test that opted out instead of against the helper, and it reaches the one skip that used to escape entirely -- `pytest.skip(allow_module_level=True)` raises at collection, and a module-level skip of `test_interactive_session.py` would have vanished thirteen tests green. **The floor's correction never reached the summary line**, though the module said twice that this was the whole point. The `F`-rewrite half did; the floor half left `4 passed, 18 skipped` as the last green line with the shortfall twenty rows above it. The shortfall is now filed as a failure report, so the last line reads `1 failed, 4 passed, 18 skipped` in red and names it. And the floor was asking the wrong question. "Did any e2e test run" reddens an honest run: `pytest -m e2e test/e2e/test_interactive_session.py` with `DEVLAUNCH_E2E_WORKSPACE` set attaches to a workspace somebody else built and correctly builds none. The question is "did the tests that promised a container produce one", and which tests promise one is now declared by them with `@pytest.mark.creates_workspace` rather than guessed at afterwards. Docs: the README printed `9 e2e tests attempted` where every run prints 22, in the paragraph teaching the reader that this is the line to read; and both the README and the workflow comment claimed "every pull request" when `pull_request: branches: [main]` means a PR onto any other base runs no CI at all. The nightly-reversal instruction was wrong for the same reason -- with `push: branches: [main]` still there, `!= 'pull_request'` is everything except PRs, not nightly. `shortfalls()` returned a list that was only ever empty or one long and whose contents the caller ignored; it is `shortfall() -> str | None` now. `devpod_available()` had one caller and is inlined into it.
44d0bbe to
e3cd0ce
Compare
All four blocking findings fixed, plus S2, S3, S4 and the module-level-skip S1 — the ledger's only door had a second one. Fixed.The identity of the runner is what the ledger now records against: _REAL_RUN = subprocess.run
...
def create_e2e_workspace(..., run=_REAL_RUN):
...
if run is _REAL_RUN:
LEDGER.record_workspace_created(workspace_id)
Pinned by S6 —
|
1 participant
Closes #114.
One
e2ejob in.github/workflows/ci.yml, plain unnestedpytest -m e2eonthe runner's own Docker. It runs wherever the rest of this workflow runs: pushes
to
main, and pull requests targetingmain. A PR onto any other base — astacked one — triggers the whole workflow not at all, which is a repo-wide gap
rather than an e2e one, and is now stated rather than papered over with "every
pull request". No dind, no devcontainer, no devpod install step —
ubuntu-latestis an ephemeral VM with Docker preinstalled, and devpod is a pixidependency already restored by
setup-pixi. Outside the py310–py313 matrix,because what the suite exercises is devpod and Docker rather than a Python
version.
Per-PR, not nightly
Per pull request, and the measurement settles it more comfortably than the
estimate did: the e2e job finishes before the matrix does — 37s against
42–52s on the final run, with the suite itself 28.67s and the 1.25 GB fixture
image pulled once for all four workspaces. It runs beside the matrix, so it adds
nothing to the wait. Against that, a nightly failure arrives detached from the
change that caused it, on a repo where nobody is rostered to triage one.
The choice is one job with one comment above it: to make it nightly, add
schedule:toon:and oneif:to the job. Nothing else moves. There is alsoa
workflow_dispatchtrigger now, so the suite can be run on demand withouteither.
timeout-minutes: 25is not boilerplate. The registry outage described below isa job that hangs for six hours without it.
The part that makes the tick mean something
A job that fires and skips its way to green is worse than one that never fires,
and this suite is unusually good at it.
skippednames two unrelated outcomeshere: thirteen
test_interactive_session.pytests declining an opt-in they werenever given, and a test that could not reach a registry. #114's comment records a
run that reported
7 passed, 14 skippedhaving created zero containers,because
mcr.microsoft.comwas serving the fixture image at 640 B/s. Thatsummary line is byte-identical to a healthy run's. Thirteen legitimate skips is
exactly the noise floor that makes the fourteenth invisible.
So the deliberate case gets a type of its own, and everything else that skips
is treated as the absence it is:
opt_out(reason)raises aDeclaredOptOut, a subclass of whatpytest.skipraises. The three sites that are genuinely opt-outs use it: noDEVLAUNCH_E2E_WORKSPACE, noclaudein the workspace, no real~/.devpodstate to be isolated from.
test/e2e/is rewritten into a failure. The hookjudges
call.excinfo— the live exception — rather than the printed report,so nothing can spell its way into an opt-out, an
xfail(which also reportsskipped, but raises the assertion that failed) passes through untouched, anda
pytest.skip(allow_module_level=True)raised at collection is caught too.Rewritten into the report, not checked at the end — the complaint is that
the summary line lies, so the correction has to land in the summary line, as
an
Fagainst the test's own name.DEVLAUNCH_E2E_WORKSPACEthat devpod cannot describe, and a missing devpod —the latter moved out of five per-test guards into one session precondition, so
a machine with no devpod on it stops reporting an e2e suite that passed.
@pytest.mark.creates_workspace; the one place that creates workspaces recordseach one, and only when the real subprocess runner did it; every run prints the
tally; and a session that ran a workspace-building test and finished with none
built does not exit zero. That is the exact quantity that was zero in the
outage run, and it is the one thing no report of a passing test can answer —
inferring a container from a green test is the mistake being fixed.
A shortfall is filed as a failure report rather than only printed, so the last
line of a run that built nothing reads
1 failed, 4 passed, 18 skippedin redinstead of leaving the green line the ticket complained about as the final word.
Two runs are not judged, because neither promised anything: one with no e2e
tests in it (
pixi run testcollects the directory and deselects it), and onewith no workspace-building tests in it —
pytest -m e2e test/e2e/test_interactive_session.pywithDEVLAUNCH_E2E_WORKSPACEsetattaches to a workspace somebody else built and correctly creates none.
Verified on the runner
8 passed, 14 skipped, 4 workspaces, 28.67s suite in a 37s job4 passed, 18 skippedand not one failure — the run that used to be green. The job is red, and says whypytest.skipis reported asFAILED, not as a skipAfter review, on the current head:
22 e2e tests attempted, 4 workspaces created: …— the number the README now prints.-ranames the test that opted out (test/e2e/test_interactive_session.py:114), where it used to name the helperpytest.skip, onexfail. Last line:2 failed, 3 passed, 18 skipped, 999 deselected, 1 xfailed— the shortfall is in it (FAILED e2e session floor), the undeclared skip is anFagainst its own test, and the xfail is still an xfail rather than the hard failure the old text-matching check would have made of itThe sabotage commits were pushed to this branch and then taken back out; the run
logs outlive them.
The first thing the job caught
test_git_status_via_sshfailed, and it is this PR's own subject in miniature.It handed devpod the fixture's bare repo — the stand-in remote its siblings
use as a clone source. devpod opens a local path as the workspace folder rather
than cloning it, so the container came up sitting inside
objects/,refs/anda
core.bare=trueconfig, wheregit statusexits 128 saying it must be run ina work tree. Not a runner quirk; it could never have been anything else, on any
machine.
It had never been observed because it had never run. Both assertions sat
behind an
if result.returncode == 0that was never true —devpod upwithout--ide noneexits 1 on any headless machine, and the test read that as "couldnot create" and moved on. #118 fixed the creation step and thereby made these
assertions reachable; the first session ever to reach them was the first CI run.
Fixed here by pointing it at the working copy, which is what a test about git in
a container needed all along, and the rc assertion now carries the output because
devpod buries git's message in its own logging.
Worth someone deciding separately:
devpod up <local path>mounts, it does notclone. Every workspace-creating test in the suite passes a filesystem path, so
none of them is exercising a clone. The other three only assert the workspace
exists, so they are not wrong — but the fixture's
remote_urlis not being usedas a remote by anything.
Proposed CHANGELOG entry
Not applied here — four PRs are in flight and four version bumps would collide.
Nothing about how you install or run
dlchanges; this release is entirelyabout what the project's own test suite does and what a green tick from it
means.
Added
mainand on pullrequests targeting
main. It isplain
pytest -m e2eagainst the runner's own Docker rather than a nesteddaemon: a development machine needs one because it is shared and long-lived,
and a runner is an ephemeral VM with Docker already on it. The job sits outside
the py310–py313 matrix, because what it exercises is devpod and Docker and not
a Python version, and it needs no devpod install step, devpod being a pixi
dependency already in the lockfile. It finishes before the matrix does.
pixi run test-e2eis the same run on your own machine — where it builds realcontainers on your Docker, so read the README first.
Fixed
unrelated outcomes were both spelled
skipped: tests declining an opt-in theywere never given, and tests that could not reach what they needed. A run
against a registry serving the suite's 1.25 GB fixture image at 640 B/s
reported
7 passed, 14 skippedhaving created no containers at all, which isthe summary line a healthy run prints — and with thirteen legitimate skips in
the baseline, one more was invisible. Deliberate skips now say so in a word of
their own — a distinct exception type, so the check is what a test raised and
not how it worded it; any other skip under the e2e directory is reported as a
failure against the test's own name; a missing devpod fails the session once
instead of skipping five tests quietly; and every run prints the workspaces it
actually built and refuses to exit zero if the tests that promised one built
none.
test_git_status_via_sshtests git in a container again. It had been pointedat the fixture's bare repository, which has no work tree, so
git statusinside the container exited 128 on any machine; it now gets the working copy.
Nobody had seen it, because until the creation
step was made unskippable its assertions sat behind a condition that was never
true on a headless machine. The first session to reach them was the first CI
run of this suite.
🤖 Generated with Claude Code