Replace legacy E2E coverage with an owner-only CLI contract suite#127
Replace legacy E2E coverage with an owner-only CLI contract suite#127
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR replaces the legacy resource-by-resource E2E suite with a single owner-only CLI contract E2E suite, centered on validating end-to-end CLI behavior against a real account.
Changes:
- Removed the legacy
e2e/testssuite and introducede2e/cli_testswith contract-style coverage (output/syntax + key workflows). - Added a shared owner-only fixture to set up common resources once per run, plus local debugging controls for fixture inspection.
- Updated
Makefile, README, and environment templates to makemake e2ethe primary workflow.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| e2e/tests/user_test.go | Removed legacy user E2E tests in favor of the new CLI contract suite. |
| e2e/tests/upload_test.go | Removed legacy upload E2E tests; coverage moved to the new contract suite. |
| e2e/tests/tag_test.go | Removed legacy tag list E2E test; covered by CLI contract suite. |
| e2e/tests/step_test.go | Removed legacy step CRUD E2E tests; replaced by contract tests. |
| e2e/tests/reaction_test.go | Removed legacy reaction E2E tests; replaced by contract tests. |
| e2e/tests/pin_test.go | Removed legacy pin E2E tests; replaced by contract tests. |
| e2e/tests/notification_test.go | Removed legacy notification E2E tests; replaced by contract tests. |
| e2e/tests/markdown_sanitization_test.go | Removed legacy markdown sanitization E2E tests; suite is now focused on CLI contracts. |
| e2e/tests/main_test.go | Removed legacy E2E TestMain; new suite provides its own TestMain flow. |
| e2e/tests/identity_test.go | Removed legacy identity E2E tests; replaced by auth/identity coverage in cli_tests. |
| e2e/tests/error_test.go | Removed legacy error-format E2E tests; new suite asserts exit codes/contracts. |
| e2e/tests/comment_test.go | Removed legacy comment E2E tests; replaced by contract tests. |
| e2e/tests/comment_attachment_test.go | Removed legacy comment attachment E2E tests; replaced by round-trip contract tests. |
| e2e/tests/column_test.go | Removed legacy column E2E tests; replaced by contract CRUD + syntax tests. |
| e2e/tests/card_test.go | Removed legacy card E2E tests; replaced by contract CRUD/workflow tests. |
| e2e/tests/board_test.go | Removed legacy board E2E tests; replaced by contract CRUD/workflow tests. |
| e2e/tests/auth_test.go | Removed legacy auth E2E tests; replaced by auth contract tests. |
| e2e/tests/attachment_test.go | Removed legacy attachment E2E tests; replaced by upload/attachment round-trip tests. |
| e2e/harness/harness.go | Added helper to report missing required env vars for the new suite. |
| e2e/harness/fixture.go | Added shared fixture setup/teardown for owner-only CLI contract runs. |
| e2e/cli_tests/webhook_test.go | Added webhook CRUD contract coverage. |
| e2e/cli_tests/utility_commands_test.go | Added config/doctor/commands/version contract coverage. |
| e2e/cli_tests/upload_attachment_test.go | Added upload + card/comment attachment round-trip coverage with downloads. |
| e2e/cli_tests/syntax_contract_test.go | Added syntax/usage contract assertions for CLI commands and flags. |
| e2e/cli_tests/search_and_auth_test.go | Added search + negative auth contract coverage. |
| e2e/cli_tests/output_contract_test.go | Added output contract coverage across common flags (--quiet, --jq, etc.). |
| e2e/cli_tests/main_test.go | New TestMain to build binary (if needed), create shared fixture, and control teardown/inspection. |
| e2e/cli_tests/helpers_test.go | Added shared helpers (resource creation, ID extraction, avatar checks, etc.). |
| e2e/cli_tests/crud_subresources_test.go | Added contract-level CRUD coverage for subresources (columns/comments/steps/reactions/notifications). |
| e2e/cli_tests/crud_card_test.go | Added card workflow and CRUD coverage (assign/pin/tag/image/attachments/move, etc.). |
| e2e/cli_tests/crud_board_test.go | Added board CRUD and workflow coverage (publish/unpublish, views, entropy, involvement). |
| e2e/cli_tests/account_user_test.go | Added account and user contract coverage including avatar update/remove. |
| README.md | Updated local testing docs to use make e2e and documented required env vars and fixture inspection modes. |
| Makefile | Replaced test-e2e behavior with e2e target, added per-file/run helpers, and optional .env.test loading. |
| .env.test.example | Added template for local E2E credentials. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed — addressed the review feedback: auth tests now use ExitAuthFailure, .env.test is gitignored and refused if tracked, avatar redirect checks accept any 3xx, and fixture configHome is only removed after successful teardown. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed — addressed the follow-up review feedback: fixture setup now does best-effort remote cleanup on setup failure, reaction CRUD discovers created reaction IDs via list readback instead of assuming create returns one, output contract tests detect JSON envelopes by decoding JSON instead of checking a prefix, and Make now respects a caller-provided FIZZY_TEST_BINARY override. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="e2e/harness/fixture.go">
<violation number="1" location="e2e/harness/fixture.go:45">
P2: Teardown can return before removing the temp config directory, so a setup failure followed by a delete error will leak the temp HOME. Consider ensuring tmpDir cleanup still happens when Teardown fails.</violation>
</file>
<file name="e2e/cli_tests/crud_subresources_test.go">
<violation number="1" location="e2e/cli_tests/crud_subresources_test.go:118">
P2: Using `listAddedID` to identify the created reaction is nondeterministic and can delete the wrong reaction when more than one new item appears between list snapshots.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Fixed — addressed the two issues identified by cubic: fixture setup failure cleanup now removes the temp HOME even if remote teardown also fails, and reaction CRUD now identifies newly created reactions by matching the added item for the current user and expected content instead of using a nondeterministic first-added diff. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fixed — addressed the latest review feedback: fixture teardown now always attempts temp-home cleanup and aggregates teardown errors, fixture runs now isolate XDG config/data/state paths under the temp fixture home, and the markdown/styled output contract checks now fail on any valid JSON output instead of only envelope-shaped objects. |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="e2e/harness/fixture.go">
<violation number="1" location="e2e/harness/fixture.go:70">
P2: Use a literal format string when returning the joined teardown error; passing dynamic text directly to `fmt.Errorf` can mangle `%` characters in stderr.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Fixed — addressed the latest issue identified by cubic: fixture teardown now wraps the joined error text with a literal format string so percent signs in stderr cannot be interpreted by fmt.Errorf. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 36 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR replaces the older resource-by-resource E2E suite with a single owner-only CLI contract suite focused on the main question we care about now: does the CLI work end to end against a real account?
The previous setup mixed a lot of deeper command-specific integration coverage with an E2E entrypoint that was harder to run and maintain. This change makes the E2E story simpler, easier to remember, and more aligned with real CLI confidence: syntax, output contracts, CRUD flows, auth smoke, uploads, attachments, exports, avatars, and other high-value owner workflows.
What changed
e2e/testswithe2e/cli_testsas the only E2E suiteFIZZY_TEST_TOKENFIZZY_TEST_ACCOUNTFIZZY_E2E_KEEP_FIXTURE=1FIZZY_E2E_TEARDOWN_DELAY=<seconds>make e2ethe primary way to run E2E locallyResult
The repo now has one clear E2E path with a simpler mental model and stronger coverage of the CLI behavior we actually want to trust before shipping.
Summary by cubic
Replaced the legacy per-resource E2E suite with a single owner-only CLI contract suite that runs against a real account. Fixes fixture teardown isolation, improves teardown error formatting, and adds strict
--quietoutput contract checks.Refactors
e2e/testswithe2e/cli_tests, organized by CLI contracts and workflows.--quietchecks; config/doctor/version; auth failure paths..env.test.example;.gitignoreignores.env.test; Makefile loads.env.test, refuses tracked secrets, and supportsFIZZY_TEST_BINARY; environment overrides are applied cleanly in the runner; global args omit--tokenwhen unset to enable missing-token tests.Migration
make e2e; run one test withmake e2e-run NAME=TestBoardList.FIZZY_TEST_TOKEN,FIZZY_TEST_ACCOUNT; optional:FIZZY_TEST_API_URL,FIZZY_TEST_BINARY.FIZZY_E2E_KEEP_FIXTURE=1andFIZZY_E2E_TEARDOWN_DELAY=<seconds>.Written for commit 86c9ca5. Summary will update on new commits.