[FEAT] Refactor integration test structure and fix CLI functionality #51
Conversation
- Created registry.ts to manage test definitions and groups for better organization and selection. - Extracted repo setup logic into repoSetup.ts for reusable temporary git repository creation. - Enhanced runIntegrationTest.ts to support interactive TUI picker and CLI flags for test execution.
📝 WalkthroughWalkthroughReplaces the legacy test runner with a CLI/TUI-driven integration test runner, adds a test registry and repo-setup utilities, updates CI and npm scripts to run integration tests, expands AGENTS.md testing docs, and removes the old TestCaseDefinition interface. Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as CLI/TUI
participant Registry as Test Registry
participant Runner as Test Runner
participant RepoSetup as Repo Setup
participant VSCode as VS Code Tests
User->>CLI: invoke runner (flags or interactive)
CLI->>Registry: request available groups/tests
Registry-->>CLI: return TEST_GROUPS / allTests
CLI->>User: show picker or accept flags
User->>CLI: select tests/groups
CLI->>Runner: runAll(selectedTests)
loop per test
Runner->>Registry: findTest(testId)
Registry-->>Runner: return TestDef
Runner->>RepoSetup: createMergeConflictRepo(...)
RepoSetup-->>Runner: repo path
Runner->>RepoSetup: writeTestConfig(repoPath,...)
Runner->>VSCode: execute testModule with workspace
VSCode-->>Runner: result (pass/fail, duration)
Runner->>RepoSetup: cleanup(repoPath)
end
Runner-->>CLI: aggregated summary (durations, failures)
CLI->>User: display results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Tip We've launched Issue Planner and it is currently in beta. Please try it out and share your feedback on Discord! Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 46-59: The README contains a broken fenced code block: an extra
closing ``` prematurely ends the first bash block after the line containing
"node out/tests/runIntegrationTest.js --list", which leaves the subsequent lines
unformatted; fix by removing that stray closing backtick so the block beginning
with "```bash" includes all listed commands through the final "npm run
test:integration -- --group perCell --group undoRedo" and end the block once
with a single closing ``` (keep the language tag 'bash' and ensure the final
closing fence is present).
In `@src/tests/repoSetup.ts`:
- Around line 16-23: The git() helper currently swallows all errors which masks
real failures; modify git(cwd: string, ...args: string[]) to accept an optional
tolerateFailure boolean (default false) and only return error.stdout when
tolerateFailure is true (used for expected failing commands like 'merge'); for
other failures throw or log the error (e.g., rethrow the error or call
console.warn/processLogger) so callers like createMergeConflictRepo explicitly
call git(..., true) for merges while other git operations fail loudly.
🧹 Nitpick comments (3)
src/tests/repoSetup.ts (2)
73-82: Fixed config path creates a shared-state coupling between test runs.
writeTestConfigalways writes to the samemergenb-test-config.jsoninos.tmpdir(). If two runner processes are invoked concurrently (e.g., local dev + CI, or a future parallel mode), they'll clobber each other's config. Consider using a unique filename per invocation (e.g., include the test id or a random suffix).Suggested: use a unique temp file
export function writeTestConfig( workspacePath: string, testName: string, params?: Record<string, unknown>, ): string { - const configPath = path.join(os.tmpdir(), 'mergenb-test-config.json'); + const configPath = path.join(os.tmpdir(), `mergenb-test-config-${testName}-${Date.now()}.json`); fs.writeFileSync(configPath, JSON.stringify({ workspacePath, testName, params })); return configPath; }
85-93:cleanupcan be simplified —fs.rmSyncwithrecursive + forcehandles both files and directories.The
statSync→ branch is unnecessary since Node.jsfs.rmSync({ recursive: true, force: true })works on files, directories, and non-existent paths alike.Simplified version
export function cleanup(dirOrFile: string): void { try { - const stat = fs.statSync(dirOrFile); - if (stat.isDirectory()) { - fs.rmSync(dirOrFile, { recursive: true, force: true }); - } else { - fs.unlinkSync(dirOrFile); - } + fs.rmSync(dirOrFile, { recursive: true, force: true }); } catch { /* ignore */ } }src/tests/registry.ts (1)
130-154: Silent drop of unrecognized IDs may confuse users.
resolveTestssilently ignores IDs that match neither a group nor a test. While the runner catches the empty-result case, a partially-valid input like--group takeAll --group typowould silently drop "typo" and run onlytakeAllwithout warning. Consider logging or collecting unmatched IDs so the runner can surface them.Suggested: return or report unmatched IDs
-export function resolveTests(ids: string[]): TestDef[] { +export function resolveTests(ids: string[]): { tests: TestDef[]; unmatched: string[] } { const result: TestDef[] = []; const seen = new Set<string>(); + const unmatched: string[] = []; for (const id of ids) { const group = findGroup(id); if (group) { for (const t of group.tests) { if (!seen.has(t.id)) { result.push(t); seen.add(t.id); } } continue; } const test = findTest(id); if (test && !seen.has(test.id)) { result.push(test); seen.add(test.id); + } else if (!test) { + unmatched.push(id); } } - return result; + return { tests: result, unmatched }; }
1 participant
Summary by CodeRabbit
New Features
Tests
Documentation
Chores