feat(test): Add smart test detection (issue #258) #263
Conversation
📝 WalkthroughWalkthroughThis PR adds specification and implementation plan documents for Issue 258's Smart Test Detection feature, outlining the design, phases, commands, acceptance criteria, and technical approach for a git-aware test runner in the core CLI. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🤖 Fix all issues with AI agents
In @.core/task/issue/258/plan.md:
- Around line 8-13: Phase 2 only maps changed .go files to a single test file
and misses renames/deletes/new files and N:M mappings; add a Phase 2.5 (or
extend Phase 2) that uses git diff --name-status (look for R for renames, A for
adds, D for deletes) to detect renames and map tests to the new path, treat
deleted source files by deciding whether to remove or still run their tests,
warn (or log) when new files lack tests, and support N:M mappings by discovering
multiple test files per source (and shared tests covering multiple sources)
rather than assuming one-to-one mapping; update the logic that builds the
file→test mapping to consult git name-status, search for multiple *_test.go
matches (not just <file>_test.go), and emit warnings for missing tests or
ambiguous/shared test files.
- Line 24: The fallback for the `--all` flag is calling the raw command `go test
./...`; change this to invoke the project's wrapper by using `core go test`
instead (preserving any needed flags/arguments), i.e., replace occurrences of
the `go test ./...` fallback with `core go test` so the `--all` path uses the
documented `core` interface; update any text or examples that mention `go test
./...` under the `--all` section to reference `core go test` and ensure the
`--all` description still indicates it skips detection before running tests.
- Around line 16-19: Phase 3 should not invoke "go test" directly; instead,
reuse the existing core implementation by calling the shared runGoTest() helper
(or invoking the logic in internal/cmd/go/cmd_gotest.go) so you keep environment
setup (MACOSX_DEPLOYMENT_TARGET, CGO_ENABLED), output filtering, coverage
parsing, JSON support and consistent styling; update the Phase 3 code to
construct the same flags mapping (--coverage -> -cover, --filter -> -run) and
pass them into runGoTest() (or refactor shared logic into a callable function if
needed), or add a short justification comment in the Phase 3 implementation
explaining why a separate execution path is necessary if you intentionally opt
not to reuse runGoTest().
- Line 9: The plan currently uses `git diff --name-only HEAD`, which only
detects local uncommitted changes; update the plan to clarify intended contexts
and implement branch-aware diff strategies: state whether Phase 2 targets local
development, CI/PR, or both, and if both make change detection configurable
(e.g., a flag or env var). Replace or add alternatives such as `git diff
--name-only origin/main...HEAD` (for PR/CI against base branch) and `git diff
--name-only HEAD` (for local uncommitted work), and document when to use each
strategy and the default behavior if none specified.
In @.core/task/issue/258/spec.md:
- Around line 17-23: The spec currently reads Go-specific (e.g., detecting `.go`
files, mapping `foo.go → foo_test.go`, using `go test`) but the PR and examples
(e.g., `src/User.php → tests/UserTest.php`) imply broader language support;
update the acceptance criteria to either (A) explicitly state this is a Go-only
feature (change command name to `core go test` and keep `git diff --name-only`,
`foo.go → foo_test.go`, `go test` mapping and flags `--all`, `--filter`,
`--coverage`) or (B) make the spec language-agnostic by adding a language
mapping section and examples (describe detection via `git diff --name-only`, a
pluggable mapping table like `Go: foo.go→foo_test.go`, `PHP:
src/*.php→tests/*Test.php`, and per-language test runner selection used by `core
test`), and note that mapping logic and test execution must be implemented per
language; pick one approach and update the acceptance criteria and examples
accordingly.
🧹 Nitpick comments (1)
.core/task/issue/258/spec.md (1)
4-4: Markdown: Wrap bare URL in angle brackets.The bare URL triggers a markdown lint warning. While minor, wrapping it in angle brackets follows markdown best practices.
📝 Proposed fix
-https://github.com/host-uk/core/issues/258 +<https://github.com/host-uk/core/issues/258>
| ## Phase 2: Change Detection | ||
| 1. Run `git diff --name-only HEAD` to get changed files | ||
| 2. Filter for `.go` files (exclude `_test.go`) | ||
| 3. Map each file to its test file: | ||
| - `internal/foo/bar.go` → `internal/foo/bar_test.go` | ||
| - Skip if test file does not exist |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add edge cases for file operations beyond simple edits.
Phase 2 handles basic file-to-test mapping but doesn't address several important scenarios:
- Renamed files: Git shows renames as delete + add; mapping should follow the rename
- Deleted files: Should corresponding tests be removed from the run, or should they run to verify cleanup?
- New files without tests: Should the tool warn that tests are missing, or silently skip?
- Files with multiple test files: Some source files may have multiple associated test files (unit, integration, etc.)
- Shared test files: Test files that cover multiple source files
Consider adding a sub-section to Phase 2 or Phase 4 that explicitly handles these cases:
## Phase 2.5: Advanced Mapping
- Detect renames via `git diff --name-status` (R flag)
- For deleted files, consider if tests should still run
- For new files, optionally warn about missing tests
- Support N:M mapping (multiple tests per file, multiple files per test)🤖 Prompt for AI Agents
In @.core/task/issue/258/plan.md around lines 8 - 13, Phase 2 only maps changed
.go files to a single test file and misses renames/deletes/new files and N:M
mappings; add a Phase 2.5 (or extend Phase 2) that uses git diff --name-status
(look for R for renames, A for adds, D for deletes) to detect renames and map
tests to the new path, treat deleted source files by deciding whether to remove
or still run their tests, warn (or log) when new files lack tests, and support
N:M mappings by discovering multiple test files per source (and shared tests
covering multiple sources) rather than assuming one-to-one mapping; update the
logic that builds the file→test mapping to consult git name-status, search for
multiple *_test.go matches (not just <file>_test.go), and emit warnings for
missing tests or ambiguous/shared test files.
| 3. Add flags: `--all`, `--filter`, `--coverage` | ||
|
|
||
| ## Phase 2: Change Detection | ||
| 1. Run `git diff --name-only HEAD` to get changed files |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "plan.md" -type f | grep -E "258|issue"Repository: host-uk/core
Length of output: 86
🏁 Script executed:
cat -n ./.core/task/issue/258/plan.mdRepository: host-uk/core
Length of output: 1308
🏁 Script executed:
# Search for any additional context about issue 258 or the test command
rg -i "issue.?258|test.*command" --type md -A 3 -B 1 | head -40Repository: host-uk/core
Length of output: 1268
🏁 Script executed:
# Check if there's any existing test command implementation or related issues
git ls-files | grep -E "(test\.go|cmd.*test)" | head -20Repository: host-uk/core
Length of output: 716
🏁 Script executed:
cat -n docs/cmd/test/index.mdRepository: host-uk/core
Length of output: 1891
🏁 Script executed:
# Look for the existing test command implementation
find . -path "*/cmd/test.go" -o -path "*/gotest.go" | head -10Repository: host-uk/core
Length of output: 38
🏁 Script executed:
# Search for the test command registration in cobra commands
rg "test.*Command|Command.*test" --type go -l | grep -E "cmd|command" | head -10Repository: host-uk/core
Length of output: 355
🏁 Script executed:
cat -n internal/cmd/test/cmd_main.goRepository: host-uk/core
Length of output: 2338
🏁 Script executed:
# Check if there's more context about issue 258 or whether this plan is being implemented
rg "258" --type md -B 2 -A 2 | head -50Repository: host-uk/core
Length of output: 38
🏁 Script executed:
# Check git history or status of this plan file
git log --oneline ./.core/task/issue/258/plan.md 2>/dev/null | head -5Repository: host-uk/core
Length of output: 108
🏁 Script executed:
# Check for CI/workflow configurations that might indicate intended use
find . -name "*.yml" -o -name "*.yaml" | grep -E "(github|gitlab|ci|workflow)" | head -10Repository: host-uk/core
Length of output: 384
🏁 Script executed:
cat -n ./.github/workflows/ci.yml | head -60Repository: host-uk/core
Length of output: 1375
🏁 Script executed:
# Check if there's any GitHub Actions workflow or CI config that uses a test change detection
rg "git diff" --type yaml --type yml -B 2 -A 2Repository: host-uk/core
Length of output: 83
🏁 Script executed:
# Check if there's any GitHub Actions workflow or CI config that uses a test change detection
rg "git diff" --type yaml -B 2 -A 2Repository: host-uk/core
Length of output: 38
🏁 Script executed:
# Check if the test command implementation has change detection logic
rg "git diff" internal/cmd/test/ -A 3 -B 3Repository: host-uk/core
Length of output: 38
🏁 Script executed:
# Check the full test command implementation
rg "runTest" internal/cmd/test/ -A 50 | head -100Repository: host-uk/core
Length of output: 4359
Clarify the context and git diff strategy for change detection.
The command git diff --name-only HEAD compares the working directory against HEAD, which works for local uncommitted changes but may not be appropriate for CI/PR contexts where you typically want to compare against the base branch.
Consider these scenarios:
- Local development:
git diff --name-only HEADshows uncommitted changes ✓ - PR/CI context: You likely need
git diff --name-only origin/main...HEADor similar to compare the PR branch against the base branch - After committing:
git diff --name-only HEADreturns nothing without additional flags
Before implementing Phase 2, clarify:
- Is this intended for local development, CI/PR workflows, or both?
- Should the change detection be optional (e.g., enabled only with a flag)?
- How should the strategy differ between local and CI contexts?
🤖 Prompt for AI Agents
In @.core/task/issue/258/plan.md at line 9, The plan currently uses `git diff
--name-only HEAD`, which only detects local uncommitted changes; update the plan
to clarify intended contexts and implement branch-aware diff strategies: state
whether Phase 2 targets local development, CI/PR, or both, and if both make
change detection configurable (e.g., a flag or env var). Replace or add
alternatives such as `git diff --name-only origin/main...HEAD` (for PR/CI
against base branch) and `git diff --name-only HEAD` (for local uncommitted
work), and document when to use each strategy and the default behavior if none
specified.
| 1. Build `go test` command with detected test files | ||
| 2. Pass through `--coverage` flag as `-cover` | ||
| 3. Pass through `--filter` as `-run` pattern | ||
| 4. Stream output to terminal |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the go test command implementation in core
rg -n "goTest|go.*test.*Command" cmd/core/ -A 10 -B 3 -t goRepository: host-uk/core
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search more broadly for test command implementation
fd -type f -name "*.go" cmd/core/ | head -20 | xargs rg -l "test" | head -10Repository: host-uk/core
Length of output: 520
🏁 Script executed:
#!/bin/bash
# Check the cmd/core directory structure
find cmd/core -type f -name "*.go" | head -20Repository: host-uk/core
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Get repository structure
ls -laRepository: host-uk/core
Length of output: 1796
🏁 Script executed:
#!/bin/bash
# Find the plan.md file
find . -name "plan.md" -type fRepository: host-uk/core
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Look for core command implementation
rg -l "core.*go.*test|goTest" --max-count 20Repository: host-uk/core
Length of output: 408
🏁 Script executed:
#!/bin/bash
# Read the plan.md file to understand Phase 3 context
cat -n .core/task/issue/258/plan.mdRepository: host-uk/core
Length of output: 1308
🏁 Script executed:
#!/bin/bash
# Examine the existing core go test command implementation
cat -n internal/cmd/go/cmd_qa.goRepository: host-uk/core
Length of output: 18776
🏁 Script executed:
#!/bin/bash
# Check for other go command files
ls -la internal/cmd/go/Repository: host-uk/core
Length of output: 554
🏁 Script executed:
#!/bin/bash
# Examine the existing core go test command
cat -n internal/cmd/go/cmd_gotest.goRepository: host-uk/core
Length of output: 11482
Align Phase 3 implementation with existing core go test infrastructure.
Phase 3 proposes building a direct go test command, but the codebase already provides core go test (in internal/cmd/go/cmd_gotest.go) which includes:
- Environment setup (MACOSX_DEPLOYMENT_TARGET, CGO_ENABLED)
- Output filtering (linker warnings)
- Consistent styling and formatting
- Coverage parsing and display
- JSON output support
Running go test directly bypasses this infrastructure. Either:
- Reuse
core go testor its underlyingrunGoTest()function internally - Document why a parallel execution path is being created
🤖 Prompt for AI Agents
In @.core/task/issue/258/plan.md around lines 16 - 19, Phase 3 should not invoke
"go test" directly; instead, reuse the existing core implementation by calling
the shared runGoTest() helper (or invoking the logic in
internal/cmd/go/cmd_gotest.go) so you keep environment setup
(MACOSX_DEPLOYMENT_TARGET, CGO_ENABLED), output filtering, coverage parsing,
JSON support and consistent styling; update the Phase 3 code to construct the
same flags mapping (--coverage -> -cover, --filter -> -run) and pass them into
runGoTest() (or refactor shared logic into a callable function if needed), or
add a short justification comment in the Phase 3 implementation explaining why a
separate execution path is necessary if you intentionally opt not to reuse
runGoTest().
| ## Phase 4: Edge Cases | ||
| - No changed files → inform user, optionally run all | ||
| - No matching test files → inform user | ||
| - `--all` flag → skip detection, run `go test ./...` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check project test organization and existing test commands
# Check module structure
echo "=== Module structure ==="
fd -e mod go.mod
echo -e "\n=== Test build tags ==="
rg -n "//go:build|// \+build" --glob "*_test.go" | head -20
echo -e "\n=== Test helper scripts/configs ==="
find . -type f \( -name "*test*" -o -name "*Test*" \) -not -path "*/vendor/*" -not -path "*/.git/*" | grep -E "\.(sh|yaml|json|txt|md)$" | head -30
echo -e "\n=== Checking for 'core go test' command definition ==="
rg "core go test|go test" --glob "*.md" --glob "*.sh" --glob "*.go" -B 2 -A 2 | head -50Repository: host-uk/core
Length of output: 2606
Use core go test instead of raw go test ./... for the --all flag fallback.
GEMINI.md explicitly states: "Agents should always prefer core commands over raw shell commands (like go test, php artisan, etc.)." The core go test command is documented as the proper interface for running Go tests with correct flags. Using go test ./... directly as a fallback contradicts this established project convention.
🤖 Prompt for AI Agents
In @.core/task/issue/258/plan.md at line 24, The fallback for the `--all` flag
is calling the raw command `go test ./...`; change this to invoke the project's
wrapper by using `core go test` instead (preserving any needed flags/arguments),
i.e., replace occurrences of the `go test ./...` fallback with `core go test` so
the `--all` path uses the documented `core` interface; update any text or
examples that mention `go test ./...` under the `--all` section to reference
`core go test` and ensure the `--all` description still indicates it skips
detection before running tests.
| ## Acceptance Criteria | ||
| - [ ] Detect changed `.go` files via `git diff --name-only` | ||
| - [ ] Map source files to test files (`foo.go` → `foo_test.go`) | ||
| - [ ] Run only relevant tests via `go test` | ||
| - [ ] Support `--all` flag to run all tests | ||
| - [ ] Support `--filter` flag for pattern matching | ||
| - [ ] Support `--coverage` flag for coverage reports |
There was a problem hiding this comment.
Clarify language scope - spec appears Go-only but PR objectives suggest multi-language support.
The acceptance criteria focus exclusively on Go files (.go files, foo.go → foo_test.go, go test), but the PR objectives include a PHP example (src/User.php → tests/UserTest.php). This suggests the feature should be language-agnostic or at least support multiple languages.
Consider:
- Should the spec explicitly state this is Go-only, or should it outline language-agnostic detection with Go as the first implementation?
- If multi-language support is intended, the mapping logic and test execution will differ significantly per language.
- The command name
core test(rather thancore go test) suggests broader language support.
🤖 Prompt for AI Agents
In @.core/task/issue/258/spec.md around lines 17 - 23, The spec currently reads
Go-specific (e.g., detecting `.go` files, mapping `foo.go → foo_test.go`, using
`go test`) but the PR and examples (e.g., `src/User.php → tests/UserTest.php`)
imply broader language support; update the acceptance criteria to either (A)
explicitly state this is a Go-only feature (change command name to `core go
test` and keep `git diff --name-only`, `foo.go → foo_test.go`, `go test` mapping
and flags `--all`, `--filter`, `--coverage`) or (B) make the spec
language-agnostic by adding a language mapping section and examples (describe
detection via `git diff --name-only`, a pluggable mapping table like `Go:
foo.go→foo_test.go`, `PHP: src/*.php→tests/*Test.php`, and per-language test
runner selection used by `core test`), and note that mapping logic and test
execution must be implemented per language; pick one approach and update the
acceptance criteria and examples accordingly.
1 participant
Summary
Implements smart test detection for
core testcommand.Task Context
See
.core/task/issue/258/for full specification and plan:Related
Closes #258
🤖 This PR is set up for AI agent implementation via fork workflow.
Summary by CodeRabbit