Repository navigation
chore: prune low-signal unit tests in favour of E2E - #40
Merged
Merged
Conversation
Delete unit tests that would not catch a bug the E2E suites miss. Keep isolated tests only for wire-protocol parsing, OAuth flows, billing math, cancellation/concurrency, data integrity and security boundaries. - aikit: 490 -> 245 tests - harness: 931 -> 500 tests - codework: 107 -> 93 tests - plugin: drop the lone contract test (covered by CLI E2E) and its test script - codework: rename test/__snapshots__ to test/__artifacts__ - harness: fix plugin.git.live reusing a fresh fake LLM per request, which replayed the same message id and raised a session_entry unique violation - AGENTS.md/CLAUDE.md: add Testing rules (E2E first, __artifacts__, no after-the-fact unit tests) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Removes unit tests that wouldn't catch a real bug the E2E suites miss, and records testing rules so new ones don't pile up again.
An isolated test was kept only if it guards something E2E can't reliably reach:
Deleted: trivial helpers, schema/shape and constructor checks, tests that restate generated catalog data, mock-heavy tests that restate the implementation, and duplicates of CLI E2E coverage.
~11k lines of test code removed.
Other changes
packages/codework/test/__snapshots__is renamed to__artifacts__, and the lint ignore glob is updated to match.plugin.git.live.test.ts:exchange()built a new fake LLM on every request, so each reply reused the same message id and the second insert failed with asession_entryunique-constraint error. The tests still passed because they only asserted on the first request. The fake is now created once per exchange.AGENTS.md/CLAUDE.md: new Testing section — no unit tests written after the fact, E2E as the primary mechanism, a repeatable artifact under__artifacts__/at the end of each E2E test, and failure modes written down first when isolation is unavoidable.Coverage to be aware of
Some deleted harness tests are now covered only by the credential-gated remote E2E: Vercel live progress, Daytona buffered output, and loading a copied driver package. They only run when
VERCEL_OIDC_TOKENandDAYTONA_API_KEYare set.Test plan
pnpm testin aikit, harness and codework passesvp checkandvp run typecheckpassCODEWORK_PLUGIN_LIVE=1plugin live suites: 12/12 pass, with the private git case run over bothgit+sshandgit+httpstest:sandbox:e2e) and aikittest:e2enot run in this PR (paid providers / real sandboxes)🤖 Generated with Claude Code