test(gitcommit): add comprehensive unit tests #29
Conversation
Add unit tests for buffer, git, and command modules. Add -- separator to git commands in CommandBuilder. Add window_stability_delay setting to config_validation.lua. Check window visibility in Buffer._insert_commit_message. Add fallback to git diff in Git.get_contextual_diff. Remove fatal string check from CommandExecutor.run.
Add EmmyLua annotations and nil-safety checks to release notes functions to ensure robust processing of commit data and version information.
Replace string shell commands and redirects with arrays in git.lua and command.lua. Replace vim.fn.system with CommandExecutor and replace vim.loop with vim.uv.
Replace string-based command generation in CommandBuilder with arrays to improve cross-platform reliability. Update CommandExecutor and associated tests to handle the new format and remove manual shell quoting logic.
Add GitUtils.path_join to handle platform-specific separators and normalization. Replace manual string concatenation in git.lua and tools/git.lua with the new helper.
Summary of ChangesHello @jinzhongjia, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request undertakes a significant refactoring of the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit tests, which is a fantastic addition for ensuring the stability and correctness of the gitcommit extension. The extensive refactoring to use command arrays instead of strings for system calls is a major improvement for security and cross-platform reliability. I've found one issue with a new utility function and a minor improvement for one of the new tests. Overall, this is excellent work that significantly improves the quality of the codebase.
| function M.path_join(...) | ||
| local parts = { ... } | ||
| if #parts == 0 then | ||
| return "" | ||
| end | ||
|
|
||
| -- Normalize all parts to use forward slashes | ||
| local normalized = {} | ||
| for i, part in ipairs(parts) do | ||
| normalized[i] = part:gsub("\\+", "/") | ||
| end | ||
|
|
||
| -- Remove leading/trailing slashes from middle parts | ||
| for i = 2, #normalized - 1 do | ||
| normalized[i] = normalized[i]:gsub("^/", ""):gsub("/$", "") | ||
| end | ||
|
|
||
| -- Join with forward slashes | ||
| local result = table.concat(normalized, "/") | ||
|
|
||
| -- Convert to Windows backslashes if on Windows | ||
| if M.is_windows() then | ||
| result = result:gsub("/", "\\") | ||
| end | ||
|
|
||
| return result | ||
| end |
There was a problem hiding this comment.
The custom path_join implementation has some bugs that can lead to incorrect paths with double separators (e.g., path_join('a/', 'b') would result in a//b). Since your project targets Neovim 0.8+, you can simplify this greatly and improve robustness by using the built-in vim.fs.joinpath. It's designed for this purpose and handles cross-platform differences correctly.
function M.path_join(...)
return vim.fs.joinpath(...)
end
| T["release_notes_log"]["returns correct command"] = function() | ||
| local result = child.lua([[ | ||
| local Command = require("codecompanion._extensions.gitcommit.tools.command") | ||
| return Command.CommandBuilder.release_notes_log("v1.0.0", "v1.1.0") | ||
| ]]) | ||
| h.eq(result[1], "git") | ||
| h.eq(result[2], "log") | ||
| h.eq(result[4], "--date=short") | ||
| h.eq(result[5], "v1.0.0..v1.1.0") | ||
| end |
There was a problem hiding this comment.
This test for release_notes_log is a bit incomplete as it doesn't check for the --pretty=format argument at index 3. You can make the test more robust by comparing the entire resulting array.
T["release_notes_log"]["returns correct command"] = function()
local result = child.lua([[
local Command = require("codecompanion._extensions.gitcommit.tools.command")
return Command.CommandBuilder.release_notes_log("v1.0.0", "v1.1.0")
]])
h.eq({
"git",
"log",
"--pretty=format:%h\x01%s\x01%an\x01%ad",
"--date=short",
"v1.0.0..v1.1.0",
}, result)
end
No description provided.