test(strategy): combine and remove redundant tests#1301
Merged
Conversation
Consolidate tests in cmd/entire/cli/strategy that exercised identical functionality into table-driven tests, and remove ones fully subsumed by others. No loss of coverage — every distinct case survives as a named subtest row. Net: 18 redundant test functions collapsed into 7 (510 deletions, 262 insertions). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a8d84d083b26
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors redundant tests in cmd/entire/cli/strategy by consolidating equivalent cases into table-driven tests and removing tests fully covered by retained cases. It is test-only and does not change production behavior.
Changes:
- Consolidates repeated agent detection, prompt attribution, token usage, session-state, and post-commit regression tests.
- Folds redundant assertions into existing tests where behavior was already covered.
- Removes duplicate migration/session struct tests with no runtime code changes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
cmd/entire/cli/strategy/common_test.go |
Combines ReadAgentTypeFromTree fallback cases into one table-driven test. |
cmd/entire/cli/strategy/manual_commit_test.go |
Merges prompt attribution tests and folds all-metadata filtering assertion into the main test. |
cmd/entire/cli/strategy/manual_commit_condensation_test.go |
Consolidates Cursor token-usage nil cases into one table-driven test. |
cmd/entire/cli/strategy/manual_commit_migration_test.go |
Removes a duplicate migration-path test. |
cmd/entire/cli/strategy/phase_postcommit_test.go |
Merges idle/ended non-active sentinel-wait regression tests. |
cmd/entire/cli/strategy/session_state_test.go |
Combines optional time field save/load tests into one table-driven test. |
cmd/entire/cli/strategy/session_test.go |
Removes redundant struct-only session tests. |
pfleidi
approved these changes
May 29, 2026
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.
https://entire.io/gh/entireio/cli/trails/457
What
Found and consolidated redundant tests in
cmd/entire/cli/strategy— tests that exercised the exact same functionality were either combined into table-driven tests or removed when fully subsumed by another test.Candidate redundancies were identified by analyzing all 37 test files in the package, then each was verified by reading the actual code before any change. Only HIGH-confidence cases (genuinely identical behavior) were acted on.
Net: 18 redundant test functions collapsed into 7 — 510 deletions, 262 insertions. No loss of coverage; every distinct case survives as a named subtest row.
Changes
common_test.goTestReadAgentTypeFromTree_*→ 1AgentType. Merged into a table-driven test. Kept_MetadataJSON_OverridesDirseparate (distinct metadata-override branch).manual_commit_test.goTestMarshalPromptAttributionsIncludingPending_*→ 1manual_commit_test.goTestCommittedFilesExcludingMetadata_AllMetadataremovedTestCommittedFilesExcludingMetadata; folded its assertion into the base test.manual_commit_condensation_test.goTestCalculateTokenUsage_Cursor*→ 1_CursorAlwaysNil.phase_postcommit_test.go_IdleSession_SkipsSentinelWait+_EndedSession_SkipsSentinelWait→ 1_NonActiveSession_SkipsSentinelWait(subtest per phase).manual_commit_migration_test.goTestMigrateShadowBranch_CommitObjectFailureremovedLastCheckpointID) and identical assertions to_MigratePathPinsAttribution.session_test.goTestSessionCheckpointCount,TestEmptySessionremovedTestSessionStruct.session_state_test.go_WithEndedAt+_WithLastInteractionTime→ 1*time.Timefield is set. Merged into_OptionalTimeFields.Deliberately left alone
Several structurally-similar tests were verified to cover genuinely different behavior and were not touched:
SafelyAdvanceLocalRef→FetchMetadataBranch→FetchV2MainFromURL) — same invariant, different production functions with distinct historical bugs.condense_skipentry-point pair andStoreAgentTypeHint/ClaimSessionStartBanner— different functions/contracts.Helper-level duplication (inline
gitRunclosures, repeated repo bootstrap) was flagged by analysis but is out of scope here — it's helper de-duplication, not redundant tests.Verification
go test ./cmd/entire/cli/strategy/→ok(38s)mise run lint→0 issues🤖 Generated with Claude Code
Note
Low Risk
Test-only refactors in the strategy package; behavior is validated by existing and merged subtests, with no runtime code changes.
Overview
This PR refactors tests only in
cmd/entire/cli/strategy: redundant cases are folded into table-driven tests or dropped when another test already covers the same behavior. Production code is unchanged.Eight
ReadAgentTypeFromTree_*tests become one table (single vs ambiguous agent dirs, no dirs). FourmarshalPromptAttributionsIncludingPendingvariants merge into one table; the all-metadatacommittedFilesExcludingMetadataassertion moves into the main test. Three CursorCalculateTokenUsagenil checks merge intoTestCalculateTokenUsage_CursorAlwaysNil. Idle and ended PostCommit sentinel-wait regressions shareTestPostCommit_NonActiveSession_SkipsSentinelWait.TestMigrateShadowBranch_CommitObjectFailureis removed as duplicate of the migrate-path pin test. TrivialSessionstruct tests and separateEndedAt/LastInteractionTimeround-trips consolidate intoTestLoadSessionState_OptionalTimeFields.Net effect: fewer test functions, same named subtest coverage; PR description cites ~510 lines removed and ~262 added with
go test ./cmd/entire/cli/strategy/passing.Reviewed by Cursor Bugbot for commit 9bda6bd. Configure here.