-
Notifications
You must be signed in to change notification settings - Fork 0
Remove the end-to-end test suite #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # The end-to-end test suite is removed | ||
|
|
||
| _Decided 2026-07-27. Supersedes the `tests/e2e`-specific claims in [0017](./0017-vitest-default-bun-only-cli-desktop.md) (paragraphs 3–4)._ | ||
|
|
||
| `tests/e2e/` is deleted in full: both the harness-driven Vitest + shell-use terminal tier (`tests/e2e/harness/`, covering `cyrusd login`/`start`/`stop`/`status`/`agents doctor` through a real PTY) and the Playwright browser suite (`tests/e2e/web/`, covering cross-peer flows — `worker-connects`, `catalog`, `thread-lifecycle`, `thread-sync`, `cold-resume`). `.github/workflows/nightly.yml` goes with it, all three jobs: the e2e run itself, build smoke (`build:web`, CLI compile), and the real-WebRTC check (`node-datachannel` against `shared/connections`) — none of the latter two were e2e-specific, but nothing else depended on the nightly workflow file existing. The tooling that only existed to run the suite goes too: `mise.toml`'s `process-compose` and `github:microsoft/shell-use` tools, and the e2e-only fixtures in `tooling/test/` (`fixtures/wrangler-env-file.ts`, `fixtures/terminal-session.ts`, `fixtures/cli-worker-state.ts`, `mocks/cli-worker-config.ts`, `helpers/process.ts`). `tooling/test/fixtures/cyrus-home.ts` and `tooling/test/mocks/auth-env.ts` stay — both are used outside `tests/e2e` too (`apps/cli`'s own unit tests, and the root `@cyrus/server` Vitest project's Miniflare bindings, respectively) — so removing them would have cut real, unrelated coverage for no reason tied to this decision. | ||
|
|
||
| The reason is scope, not fidelity. The suite's entire cost — a compiled `cyrusd` binary rebuilt fresh on every run, a `wrangler dev` + local D1 + Vite stack orchestrated through process-compose, a PTY driver for the terminal tier, Playwright browsers — was carried off the path every contributor actually feels: it ran only on manual `workflow_dispatch`, never on PRs or pushes to `main`, and cron was deliberately deferred pending "a stable pass history" (per the now-superseded `docs/guides/TESTING_FRAMEWORK.md` text) that it never reached in its short life. Real infrastructure cost, zero automatic runs, no established reliability track record: that combination is pure maintenance surface without a safety net anyone could actually depend on day to day. Nothing about the approach itself — real process orchestration over mocking, PTY-driven terminal assertions, Playwright for cross-peer browser flows — was found lacking; this trades that fidelity for not carrying its cost right now. | ||
|
|
||
| Some OpenSpecs lose their only cross-process/browser-level coverage as a result: `acp-provider-cli`, `acp-session-router`, `connection-providers`, and `conversation-persistence` had it in the removed terminal tier or Playwright specs (see the prior `docs/guides/TESTING_FRAMEWORK.md` OpenSpec coverage map) and now rely solely on their existing unit/integration tests. `docs/guides/VERIFY_LOOP.md`'s manual full-stack loop — starting the server, web controller, and CLI worker by hand and driving the browser with `playwright-cli` — is now the only way to exercise these flows end to end, where `bun test:e2e` previously gave a repeatable automated substitute. | ||
|
|
||
| If a future need re-justifies this cost — e.g. a recurring class of bug that only manifests across the real process boundary — the removed harness is fully recoverable from git history (branch `chore/e2e-harness-cleanup`, the session that did this removal). A fresh implementation should reconsider the fidelity-vs-cost tradeoff from scratch rather than resurrect the same shape verbatim, given it never reached the "stable pass history" bar this ADR's predecessor was written against. | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.