Test improvements: isolation, review findings, version mismatch warning #136
+385
−53
Conversation
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
- Track locationLineLen and compute wrapped line count for header height calculation, fixing scroll indicator accuracy with long paths/branches - Add TestTUIRenderReviewViewAddressedWithoutVerdict for line ordering - Update TestTUIVisibleLinesCalculationLongTitleWraps for new wrapping Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Use runewidth.StringWidth instead of len() for titleLen and locationLineLen to correctly handle non-ASCII characters - Add upfront line count check in TestTUIRenderReviewViewAddressedWithoutVerdict to fail fast instead of silently passing with too-short output Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add setupTuiTestEnv helper to set ROBOREV_DATA_DIR to temp directory, preventing TUI tests from reading production daemon.json - Add testServerAddr constant to replace hardcoded localhost:7373 - Fix TestDaemonRunStartsAndShutdownsCleanly to properly stop daemon using SIGTERM via ListAllRuntimes() to find daemon PID - Remove os.Exit(0) from daemon shutdown handler to allow proper test cleanup (Go 1.25 panics on os.Exit during tests) This prevents test daemons from polluting production roborev environment and ensures tests don't leave zombie daemon processes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
wesm
changed the title
- Use os.Interrupt instead of syscall.SIGTERM for cross-platform support - Guard against signaling non-self PIDs (daemon runs in test process) - Remove force-kill fallback that could kill the test process - Call setupTuiTestEnv(t) in TestTUIConfigReloadFlash and TestTUIReconnectOnConsecutiveErrors to isolate from production env Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove self-PID check to handle future refactors where daemon may run as separate process - Change timeout from t.Log to t.Fatal to ensure test failures don't leak daemon goroutines into subsequent tests Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When the TUI connects to a daemon running a different version, display a persistent red error banner: "VERSION MISMATCH: TUI X != Daemon Y - restart TUI or daemon". This prevents confusing behavior when stale binaries are used (e.g., from baked-in post-commit hook paths). - Add versionMismatch field to tuiModel - Check for mismatch when receiving daemon status - Display error banner in queue and review views - Add TestTUIVersionMismatchDetection with 4 test cases Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
wesm
changed the title
wesm
changed the title
- Use a specific port (17373) instead of port 0 which doesn't work correctly with FindAvailablePort - Wait for daemon to be HTTP-responsive before attempting shutdown - Explicitly signal os.Getpid() with comment explaining why PID reuse is not a concern (daemon runs in same process as test) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Filter runtimes by PID == os.Getpid() to avoid matching stale runtime files from other processes - Check and fail on proc.Signal error - Clarify comment about FindAvailablePort auto-incrementing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Race detector adds significant overhead which can cause timeout failures on slower CI machines. Increase timeouts from 5s to 10s and add more context to failure messages for debugging. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
Test isolation
setupTuiTestEnvhelper to isolateROBOREV_DATA_DIRto temp directorytestServerAddrconstant to replace hardcodedlocalhost:7373os.Interruptand fail on timeoutos.Exit(0)from daemon shutdown (Go 1.25 compatibility)Review findings addressed
formatAgentLabelhelper for agent+model formattingTestGetReviewByJobIDIncludesModel)runewidth.StringWidthfor non-ASCII display widthVersion mismatch detection
Test plan
🤖 Generated with Claude Code