Add thorough tests for git change detection and utils#67
Add thorough tests for git change detection and utils#67DFanso merged 1 commit intoDFanso:mainfrom Muneer320:main
Conversation
WalkthroughAdds new Go test files for git operations, utility functions, and types. Tests include repository detection, change listing, error cases outside repos, path normalization, text/small file checks, empty filtering, JSON marshalling, and prompt content verification. Some tests run in parallel and use temporary directories; one integration-style test is skipped in short mode. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (3)
pkg/types/types_test.go (1)
9-34: Consider unmarshalling for structural verification.The test checks for literal key strings in the JSON output. While this provides basic coverage, unmarshalling the JSON back into a Config struct and verifying the values would be more robust and less brittle.
Example refactor:
- jsonStr := string(data) - if !strings.Contains(jsonStr, "\"grok_api\"") { - t.Fatalf("expected grok_api key in JSON: %s", jsonStr) - } - if !strings.Contains(jsonStr, "\"repos\"") { - t.Fatalf("expected repos key in JSON: %s", jsonStr) - } + var decoded Config + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("json.Unmarshal failed: %v", err) + } + + if decoded.GrokAPI != cfg.GrokAPI { + t.Fatalf("GrokAPI mismatch: got %q, want %q", decoded.GrokAPI, cfg.GrokAPI) + } + if len(decoded.Repos) != len(cfg.Repos) { + t.Fatalf("Repos count mismatch: got %d, want %d", len(decoded.Repos), len(cfg.Repos)) + }internal/git/operations_test.go (1)
55-111: Good integration test with one optional enhancement.The test properly skips in short mode and creates a realistic repository state with unstaged, staged, and untracked changes. Section headers are verified, but you could strengthen the test by asserting specific diff content.
Example enhancement to verify unstaged diff content:
fragments := []string{ "Unstaged changes:", "tracked.txt", // file name in unstaged section "+updated version", // diff content showing the change "Staged changes:", "Untracked files:", "Content of new file new.txt:", "Recent commits for context:", "brand new file", }internal/utils/utils_test.go (1)
10-33: LGTM!Well-structured table-driven test with parallel execution. Covers Windows-style paths, already-normalized paths, and no trailing slash scenarios.
Optional: Add edge cases for more thorough coverage:
{name: "forward slash trailing", input: "foo/bar/", expected: "foo/bar"}, {name: "empty string", input: "", expected: ""}, {name: "root path", input: "/", expected: ""},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/git/operations_test.go(1 hunks)internal/utils/utils_test.go(1 hunks)pkg/types/types_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pkg/types/types_test.go (2)
pkg/types/types.go (2)
Config(4-7)RepoConfig(10-13)pkg/types/prompt.go (1)
CommitPrompt(3-19)
internal/utils/utils_test.go (1)
internal/utils/utils.go (4)
NormalizePath(10-16)IsTextFile(19-35)IsSmallFile(38-47)FilterEmpty(50-58)
internal/git/operations_test.go (2)
internal/git/operations.go (2)
IsRepository(16-23)GetChanges(26-130)pkg/types/types.go (1)
RepoConfig(10-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Go Binary (windows-latest)
🔇 Additional comments (7)
pkg/types/types_test.go (1)
36-50: LGTM!The test verifies that CommitPrompt contains essential instructional fragments. Note that checking for literal strings makes the test brittle to prompt wording changes, but this is acceptable for a basic content validation test.
internal/git/operations_test.go (3)
13-53: LGTM!Comprehensive coverage of IsRepository with positive (initialized repo) and negative cases (non-repo directory, missing path). Proper use of t.Parallel(), t.TempDir(), and graceful skipping when git is unavailable.
113-121: LGTM!Correctly verifies that GetChanges returns an error when called on a non-repository directory.
123-133: LGTM!Well-structured helper with t.Helper() marker and GIT_TERMINAL_PROMPT=0 to prevent interactive prompts during tests.
internal/utils/utils_test.go (3)
35-58: LGTM!Correctly tests text file detection by extension, including case-insensitivity ("README.MD"), binary extensions, and files without extensions.
60-95: LGTM!Thorough test using actual temporary files to verify the 10KB size threshold. Correctly tests small files (1KB), large files (11KB), and missing files.
97-114: LGTM!Correctly verifies that FilterEmpty removes only empty strings while preserving whitespace strings (
), which aligns with the implementation.
DFanso
added
enhancement
2 participants
Description
Strengthens the test safety net by adding focused unit and integration coverage for git change detection, file heuristics, and shared prompt types.
Type of Change
Related Issue
Fixes #2
Changes Made
NormalizePath,IsTextFile,IsSmallFile, andFilterEmptyininternal/utilsGetChangescoverage ininternal/gitCommitPromptintegrity inpkg/typesTesting
Checklist
Screenshots (if applicable)
Not applicable
Additional Notes
Targeted packages now reach roughly 92% statement coverage (
go tool cover -func coverprofile), giving confidence that git change harvesting and file heuristics behave as expected.For Hacktoberfest Participants
Summary by CodeRabbit