Refactor test suite for consistency and maintainability - #55
Merged
Conversation
- Consolidate Prune behavior tests into a single table-driven test (TestPruner_PruneScenarios) to reduce boilerplate and improve readability. - Improve sub-test naming in TestFormatBytes to describe inputs unambiguously. - Tighten assertions in TestWriteSummary by sorting project output alphabetically to guarantee deterministic results, and asserting against an exact string literal instead of loose substring matching.
- Remove unused helpers writeConfigRaw and readConfigFile - Extract 'config.json' into a file-level constant 'configFileName' - Unify setupConfigDir and configWithTmpDir into setupTestEnv - Convert directory resolution tests to table-driven format
- Add readConfigFile helper function to read and unmarshal configuration files in tests. - Update TestCursorSecret_GeneratedAndPersisted, TestCursorSecret_RegeneratedIfMissing, and TestCursorSecret_PreservesOtherFields to use the readConfigFile helper. - Remove redundant configPath variable in TestCursorSecret_RegeneratedIfMissing.
- Extract insertConversation helper to automatically generate test sessions and message sequences. - Add testClock for advancing timestamps. - Implement assertEq generic helper to reduce assertion boilerplate. - Split massive TestGetAnalyticsVelocity into TestGetAnalyticsVelocity_Metrics, TestGetAnalyticsVelocity_EdgeCases, and TestGetAnalyticsVelocity_ToolUsage to improve maintainability.
- Use go-cmp for struct comparisons in TestGetSessionFull to improve readability and debuggability. - Introduce msgBuilder to encapsulate ordinal tracking in test setups, simplifying functions like setupPruneData. - Create and use requireNoError helper to consolidate repetitive err != nil boilerplate. - Add github.com/google/go-cmp/cmp dependency.
- Fix time-dependent flaky test TestSessionFilterActiveSince by explicitly setting CreatedAt. - Replace filterWith functional builder with direct SessionFilter struct initialization to reduce boilerplate. - Standardize TestActiveSinceUsesEndedAtOverStartedAt to table-driven tests for consistency.
- Replace manual map iteration and value checks with `maps.Equal` in `TestSkippedFiles_RoundTrip` for exact matching. - Fix unnecessary line wraps across `internal/db/skipped_test.go` to improve readability and adhere to standard `gofmt` idioms.
- Add missing nil guards in TestInsights_InsertDateRange and TestInsights_InsertAndGet. - Strengthen filter assertions in TestInsights_ListWithFilters by checking returned content instead of just length. - Use cmp.Diff for comprehensive struct comparison in TestInsights_InsertAndGet.
…d helper - Refactored ParseCodexStream and ParseStreamJSON tests to table-driven format - Used multiline raw string literals for JSONL payloads to reduce string concatenation noise - Extracted common createMockBinary helper to deduplicate file system setup between fakeClaudeBin and fakeGeminiBin
Consolidate fragmented test functions into a single table-driven test to eliminate boilerplate and improve readability.
Extract setup boilerplate into parseAndGetToolCalls helper and use table-driven tests to simplify claude_subagent_test.go.
- Extracted repetitive parsing and validation logic into parseAndValidateHelper - Consolidated Object, Array, and EmptyString tool result tests into a single table-driven test - Introduced a generic assertEqual helper to reduce assertion boilerplate
- Extract parseTestContent helper for standardizing test setup and validation - Consolidate time comparisons using the new formatTime helper - Clean up repetitious code to improve test readability
- Added missing error assertion in TestLineReader happy path - Replaced custom errAfterReader mock with standard io.MultiReader and iotest.ErrReader - Simplified slice comparisons using slices.Equal
- Introduce OpenCodeSeeder to abstract database setup and raw SQL inserts - Consolidate test database creation into a newTestDB helper - Add generic assertEq helper to reduce repetitive boilerplate
…fy/assert - Break TestParseClaudeSession, TestParseCodexSession, and TestParseGeminiSession into smaller domain-specific subtests in dedicated _test.go files. - Introduce github.com/stretchr/testify/assert and require to replace verbose manual condition checks. - Extract complex testjsonl.JoinJSONL constructions into static .jsonl and .json fixtures in internal/parser/testdata/. - Unify temporary file creation and test boilerplate via dedicated test runner helpers.
Renamed `name` to `toolName` in `TestNormalizeToolCategory` to separate the input variable from the test identifier, and added a fallback test name for empty string inputs to improve output readability.
- Update assertLogContains and assertLogNotContains to batch missing/unexpected substrings and print the full log string only once on failure. - Simplify assertToolCalls by extracting single struct comparison to assertToolCall.
- Rename `results` field in TestInferRelationshipTypes struct to `inputs` to clearly distinguish it from expected outputs (`want`). - Add a length validation check before iterating in TestInferRelationshipTypes to prevent out-of-bounds panics if test cases are misconfigured.
- Replace string concatenation with 'buildURL' and 'buildURLWithRange' helpers.
- Convert repetitive imperative test runs into table-driven tests.
- Abstract base API paths to a package-level constant ('basePath').
- Link test assertions to the returned seed database stats.
- Introduce 'requiresFTS' boolean field in test case struct to explicitly declare FTS dependency. - Remove hardcoded 'Search' handler check in test runner loop.
- Replace hardcoded "GET" method strings with `http.MethodGet` constant. - Add `t.Parallel()` to top-level test and subtests for concurrent execution.
- Fix risky test cleanup in makeUnreadableDir by moving cleanup registration before directory mutation. - Make 'NotExist' test paths cross-platform by using t.TempDir() instead of a hardcoded unix path. - Modernize octal literals to use the '0o' prefix. - Consolidate line breaks in helper functions to improve readability. - Enable parallel test execution for TestSyncIfModified_CacheClearing.
- Consolidate fragmented GitHub API mock tests into table-driven `TestCreateGist` and `TestValidateGithubToken` suites. - Enhance `stubServer` helper to perform `Authorization` header validation and accept `*testing.T`. - Introduce `assertErrorContains` helper to standardize error string checks and reduce boilerplate.
- Fix destructive body reads in `isTimeoutResponse` and `assertTimeoutResponse` to avoid corrupting response state and ignoring I/O errors. - Rename `newTestContext` to `newTestRequest` to align with standard Go idioms where 'context' implies 'context.Context'. - Consolidate `testServer` and `testServerOpts` to reduce API duplication, leveraging Go's variadic arguments.
- Extract shared listInsightsResponse struct for better readability. - Consolidate generate validation logic in TestGenerateInsight_Validation. - Group missing resource tests into TestInsight_ResourceErrors.
…mplify assertions - Unify TestRoutesTimeoutWiring subtests into a single table-driven test using a shared httptest.Server to reduce overhead. - Simplify Content-Type header assertion logic in TestContentTypeWrapper.
- Replace `t.Fatalf` with `t.Errorf` to prevent premature test termination in `TestParseIntParam`. - Make HTTP status expectations explicit in `TestParseIntParam` by renaming `wantErr` to `wantStatus` and enforcing `http.StatusOK` on success. - Simplify `TestClampLimit` table by removing the redundant `defaultLimit` field.
…values Refactors analytics tests (specifically TestGetAnalyticsActivity and TestGetAnalyticsHeatmap) to use the dynamically returned 'seedStats' from 'seedAnalyticsData()' for asserting totals, rather than relying on hardcoded numbers.
Replaces hardcoded "POST" and "GET" strings with http.MethodPost and http.MethodGet constants when creating new HTTP requests in export.go.
Verified that a hardcoded integer has been replaced with defaultLimit / 2 to improve test maintainability. Tests run successfully and linters pass.
…nd support multiple data lines Updates the parseSSE test helper to properly flush and append the final parsed event if it contains data, even if it lacks an explicit event name and correctly handle multiple data lines.
This commit addresses the following findings in internal/server/middleware_test.go: - Reverts `t.Errorf` and early return back to `t.Fatalf` inside `t.Run` closures for io.ReadAll failures, as `t.Fatalf` correctly aborts only the current subtest. - Reverts `!isTimeoutResponse` to `assertTimeoutResponse` to preserve detailed test failure output (mismatched status codes or body errors) instead of a generic boolean check error.
Remove redundant state assertions with nil callbacks and manual message count checks in engine_integration_test.go, deferring to the existing assertSessionMessageCount helper instead.
…eIntegration This change was already applied in a previous commit (4924ffe). Verified that no other redundant TotalSessions assertions remain.
Replaces checks on the transient w.pending map with reliable synchronization mechanisms (channel events and fsnotify watch list state) to fix flaky watcher tests.
…ields Converts an unkeyed test case struct literal to use keyed fields, intentionally omitting empty string values for cleaner initialization.
- Replaced manual assertions for ToolCalls properties with assertToolCalls in codex_parser_test.go. - Removed redundant length assertions in opencode_test.go. - Added explicit assertToolCalls check in claude_parser_test.go for improved test coverage.
- Extended seedStats struct returned by seedAnalyticsData helper with TotalUserMessages, TotalAssistantMessages, and ActiveDays. - Updated test assertions in TestGetAnalyticsActivity, TestGetAnalyticsSummary, and TestGetAnalyticsHeatmap to dynamically validate these values. - Added dynamic validation for stats.TotalMessages in TestGetAnalyticsProjects.
Update TestCORSAllowMethods to use net/http constants for HTTP methods instead of string literals.
This fixes an issue where subsequent lines of a multi-line SSE data payload were dropped and ensures proper flushing of final events on empty lines.
…rtions Removed manual UserMessageCount checks in engine_integration_test.go which also acted as redundant nil session state assertions, as they were immediately preceded by assertSessionMessageCount which already implicitly asserts that the session state exists.
This empty commit confirms the prior removal of a redundant TotalSessions assertion from the codebase. Code compilation, tests, and linting have been verified to still pass.
…lper Update assertToolCall to also verify SubagentSessionID if populated. Refactor TestSubagentSessionIDMapping to define wantTools explicitly instead of using manual map loops and length checks.
…ests Replaces hardcoded expected values with dynamic fields from the seedStats struct in analytics tests (e.g. TestAnalyticsHeatmap, TestAnalyticsTopSessions). Also updates the assertion for total messages across projects in TestAnalyticsProjects to explicitly state 'total messages across projects'.
Replaces hardcoded validation for analytics tests (such as total messages and active days) with dynamic checks against seeded stats data. Fixes inaccurate active days expected value to align with actual seed data structure, and ensures heatmap tests properly count all expected date grid entries.
Replaces string literals for HTTP methods (e.g. "POST", "DELETE") with standard net/http constants (http.MethodPost, http.MethodDelete) in server tests.
Restores the UserMessageCount validation that was previously lost when assertSessionState calls were removed from synchronization tests. Re-introducing assertSessionState with an explicit check ensures we correctly verify the breakdown of user vs. assistant messages alongside the overall total.
Empty commit confirming the removal of a redundant TotalSessions assertion.
Removes the conditional check for `want.SubagentSessionID != ""` in `assertToolCall` so that tests correctly fail if a tool call unexpectedly populates the SubagentSessionID field when it should be empty.
Updates `TestAnalyticsTopSessions` to correctly assert the number of returned sessions by accounting for the hardcoded limit of 10 in `GetAnalyticsTopSessions`. Also adds an assertion to ensure that filtered requests return at least one session, rather than silently passing if the response is empty.
Enhances analytics tests to dynamically validate total messages, sessions, and active days against seeded statistics instead of relying on hardcoded values.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
roborev: Combined Review (
|
cursor Bot
pushed a commit
to diazMelgarejo/periscope
that referenced
this pull request
Jun 1, 2026
## Summary - Split monolithic parser tests (`parser_test.go`) into per-agent files (`claude_parser_test.go`, `codex_parser_test.go`, `gemini_parser_test.go`) - Convert many tests to table-driven format - Add test fixture files under `internal/parser/testdata/` - Refactor watcher tests to use real fsnotify events instead of mocking internal state - Make analytics test assertions dynamic (derived from seed data instead of hardcoded) - Small production code improvements: deterministic prune output, http.Method constants in export.go, error handling on io.ReadAll ## Test plan - [ ] `make test` passes - [ ] `make vet` passes - [ ] No coverage regressions (test count increased from 23 to 33 parser functions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <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
1 participant
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
parser_test.go) into per-agent files (claude_parser_test.go,codex_parser_test.go,gemini_parser_test.go)internal/parser/testdata/Test plan
make testpassesmake vetpasses🤖 Generated with Claude Code