feat(test): add BATS unit tests and smoke tests for review loops#3
Merged
Conversation
Owner
rlorenzo
commented
Mar 21, 2026
rlorenzo
commented
- BATS tests cover config parsing, prompt loading, agent flags, MCP config, and review status checks
- Smoke tests run real agents as editor/reviewer against a fixture repo to catch CLI integration issues
- Pre-commit runs unit tests; pre-push runs smoke tests
- Shared helpers moved from bin/ scripts to lib/lib-review-loop
- BATS tests cover config parsing, prompt loading, agent flags, MCP config, and review status checks - Smoke tests run real agents as editor/reviewer against a fixture repo to catch CLI integration issues - Pre-commit runs unit tests; pre-push runs smoke tests - Shared helpers moved from bin/ scripts to lib/lib-review-loop
There was a problem hiding this comment.
Pull request overview
Adds a BATS-based unit test suite plus agent-backed smoke tests to validate review-loop behavior end-to-end, and wires them into local git hooks.
Changes:
- Introduces BATS unit tests covering config parsing, prompt loading, validation, and review status helpers.
- Adds
test/smoketo run real agents against a temporary fixture repo for CLI integration checks. - Moves common helper logic into
lib/lib-review-loopand updates hooks/docs accordingly.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_helper.bash |
Shared BATS setup/teardown and helper to source the library cleanly. |
test/smoke |
New smoke-test runner that exercises real agents as editor/reviewer against a fixture repo. |
test/run |
Convenience wrapper to run all .bats tests via the vendored BATS submodule. |
test/plan-review-loop.bats |
CLI argument/validation tests for bin/plan-review-loop. |
test/lib-review-loop.bats |
Unit tests for library functions (validation, config, prompt parsing, review status). |
test/fixtures/review-issues.md |
Fixture review representing “needs work” with issue counts. |
test/fixtures/review-clean.md |
Fixture review representing “good to go” with zero counts. |
test/fixtures/prompt-with-frontmatter.md |
Prompt fixture containing YAML frontmatter. |
test/fixtures/prompt-no-frontmatter.md |
Prompt fixture without frontmatter. |
test/fixtures/prompt-blank-lines.md |
Prompt fixture for trimming leading/trailing blank lines. |
test/code-review-loop.bats |
CLI argument/validation tests for bin/code-review-loop. |
test/bats/bats-support |
Adds BATS helper library as a git submodule pointer. |
test/bats/bats-core |
Adds BATS core as a git submodule pointer. |
test/bats/bats-assert |
Adds BATS assert library as a git submodule pointer. |
lib/lib-review-loop |
Centralizes review status helpers and plan improvement prompt builder. |
bin/plan-review-loop |
Updates supported agent list and removes in-script duplicated helpers. |
bin/code-review-loop |
Updates supported agent list and removes in-script duplicated helpers. |
README.md |
Documents running unit tests and smoke tests, including submodule setup. |
.pre-commit-config.yaml |
Adds local hooks: unit tests on pre-commit, smoke tests on pre-push. |
.markdownlint-cli2.yaml |
Excludes test fixtures from markdownlint. |
.gitmodules |
Declares BATS submodules used under test/bats/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
test/smoke:1
- The timeout watchdog kills only the direct agent PID; if the agent spawns children (common for CLIs), those processes may continue running after a timeout. Prefer running the agent in its own process group/session and killing the whole group on timeout, and/or escalate to SIGKILL after a short grace period if it’s still alive.
#!/usr/bin/env bash
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…rmat_elapsed test Prevents smoke tests from failing when commit.gpgsign=true is set globally. Allows 1-second tolerance in format_elapsed assertion to avoid flaky test from clock skew between date calls.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.