test: robustify conventional merge commit#10
Conversation
- Rename SCRIPT_DIR to TEST_SCRIPT_DIR for clarity - Remove redundant comments from colors.sh
Move test_enhance_scope function to reusable helper file. Helper defines its own HOOK path via BASH_SOURCE.
Add test-examples.sh demonstrating basic hook usage. Tests body preservation for both hooks.
- Support lowercase 'merge ...' messages - Support bare 'Merge' and 'merge' keywords - Add comprehensive edge case tests
Handle Merge, Merged, Merges, Squash, Squashed, Squashes (case-insensitive first letter) with optional leading whitespace and separator chars (space, (, !, :, [). Trigger pattern: ^\s*([Mm]erge[ds]?|[Ss]quash(e[ds])?)[(!:\[]?( .*)?$
Create conventional-merge-commit-helper.sh with unified test_conventional_merge_commit() function supporting both first-line-only and body-preservation tests. Update test-examples.sh to use shared helper.
Add exhaustive test cases for Merge/Squash variants covering keywords, separators, arguments, and leading whitespace. Parallelize CSV tests using background jobs (8 concurrent) with early termination on first failure. Exclude CSV from trailing-whitespace hook (test data includes intentional trailing spaces).
|
🎉 Welcome to the Thank you for your contribution! We appreciate your time and effort in submitting this pull request. A maintainer will review it soon. If you have any questions, feel free to ask! Thank you for your cooperation 🙏 |
Summary of ChangesHello @michen00, 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 focuses on enhancing the reliability and testability of the commit message hooks. It significantly upgrades the Highlights
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
|
There was a problem hiding this comment.
Code Review
This pull request significantly improves the robustness of the conventional-merge-commit hook and its tests. The test suite is now much more comprehensive with CSV-driven parallel tests, and the test scripts have been nicely refactored into shared helpers. However, I've found a critical issue in the core logic of the conventional-merge-commit script that will cause some of the new tests to fail. I've also noticed a minor issue in one of the new example tests. My feedback focuses on these two points.
There was a problem hiding this comment.
Pull request overview
This PR robustifies the conventional-merge-commit hook by expanding its trigger pattern and comprehensively testing edge cases. The hook now handles various forms of merge and squash commit messages (e.g., "Merged", "Merges", "Squashed", "Squashes") with different punctuation patterns, and includes extensive test coverage.
Key changes:
- Expanded trigger pattern to match merge/squash variants: Merge[d|s], Squash[ed|es] with case-insensitivity and leading whitespace handling
- Added
720288 CSV-driven test cases executed in parallel for comprehensive coverage - Refactored test infrastructure with reusable helper functions for both hooks
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/conventional-merge-commit | Enhanced trigger pattern to handle merge/squash variants with punctuation (!, :, [, () and leading whitespace |
| tests/test-conventional-merge-commit.sh | Refactored to use CSV-driven tests (720 cases) with parallel execution and manual edge case tests |
| tests/test-conventional-merge-commit.csv | New CSV file with 720 test cases covering all merge/squash keyword variants with different punctuation and whitespace |
| tests/conventional-merge-commit-helper.sh | New shared helper function for testing conventional-merge-commit hook with body preservation checks |
| tests/enhance-scope-helper.sh | New shared helper function for testing enhance-scope hook, extracted from test script |
| tests/test-examples.sh | New basic usage test file demonstrating both hooks with body preservation |
| tests/test-enhance-scope.sh | Refactored to use shared helper function, removed duplicate body preservation test |
| tests/test-lib-commit-msg.sh | Standardized variable naming (TEST_SCRIPT_DIR) and simplified path resolution |
| tests/test-runner.sh | Added test-examples.sh to test suite and standardized variable naming |
| tests/colors.sh | Removed redundant comments at file header |
| .pre-commit-config.yaml | Excluded CSV test file from trailing-whitespace check to preserve intentional whitespace in test data |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 663b7134a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on February 5
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
Allow commit_source=squash (from git merge --squash) to reach the transformation logic, not just commit_source=merge. Add test for squash commit source transformation.
Add optional expected_body parameter to test_enhance_scope helper (matching test_conventional_merge_commit pattern). The test named "Commit body preserved" now actually verifies body content is preserved, not just the first line.
…-conventional-merge-commit
Modified test helpers to clarify expected_body parameter usage and ensure full body content is captured from line 3 onward. Updated example test cases to reflect these changes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Michael I Chen <michael.chen@aicadium.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Michael I Chen <michael.chen@aicadium.ai>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Michael I Chen <michael.chen@aicadium.ai>
Introduced a dedicated function to handle killing background jobs, improving the cleanup process in the test script.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Michael I Chen <michael.chen@aicadium.ai>
^\s*([Mm]erge[ds]?|[Ss]quash(e[ds])?)\b.*$ roughly^\s*([Mm]erge[ds]?|[Ss]quash(e[ds])?)[(!:\[]?( .*)?$,Note
Improves commit message automation and test coverage.
scripts/conventional-merge-committo handle leading whitespace and a broader set of merge/squash variants via case patterns; rewrites tochore: ...while preserving bodies720288 cases) plus shared helpers; refactorsenhance-scopetests and adds basic usage examples; updates test runner.pre-commit-config.yamlto exclude the large CSV from thetrailing-whitespacehookWritten by Cursor Bugbot for commit 663b713. Configure here.