fix(spec): address CodeRabbit review comments on plan and spec
- Plan: clarify git diff strategy (local HEAD vs CI origin/dev...HEAD) - Plan: add Phase 2 rename/delete/add handling via git diff --name-status - Plan: add N:M test file discovery (not just 1:1 mapping) - Plan: align Phase 3 with existing runTest() infrastructure - Plan: replace raw `go test ./...` fallback with runTest() call - Plan: correct file paths to internal/cmd/test/ (not cmd/core/cmd/) - Spec: explicitly scope as Go-only with note on future language support - Spec: wrap bare URL in angle brackets - Spec: add --base flag for CI/PR context - Spec: update acceptance criteria to match revised plan - Spec: add technical context pointing to existing infrastructure Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Snider
parent
a126063aee
commit
e0c5354922
2 changed files with 57 additions and 33 deletions
|
|
@ -1,33 +1,50 @@
|
|||
# Implementation Plan: Issue 258
|
||||
|
||||
## Phase 1: Command Structure
|
||||
1. Create `cmd/core/cmd/test.go`
|
||||
2. Register `test` command with Cobra
|
||||
3. Add flags: `--all`, `--filter`, `--coverage`
|
||||
1. Extend existing `internal/cmd/test/cmd_main.go` with smart detection flags
|
||||
2. Add flags: `--all`, `--filter` (alias for `--run`)
|
||||
3. Existing flags (`--coverage`, `--verbose`, `--short`, `--race`, `--json`, `--pkg`, `--run`) are already registered
|
||||
|
||||
## Phase 2: Change Detection
|
||||
1. Run `git diff --name-only HEAD` to get changed files
|
||||
## Phase 2: Change Detection
|
||||
1. Determine diff strategy based on context:
|
||||
- **Local development** (default): `git diff --name-only HEAD` for uncommitted changes, plus `git diff --name-only --cached` for staged changes
|
||||
- **CI/PR context**: `git diff --name-only origin/dev...HEAD` to compare against base branch
|
||||
- Auto-detect CI via `CI` or `GITHUB_ACTIONS` env vars; allow override via `--base` flag
|
||||
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
|
||||
3. Use `git diff --name-status` to detect renames (R), adds (A), and deletes (D):
|
||||
- **Renames**: Map tests to the new file path
|
||||
- **Deletes**: Skip deleted source files (do not run orphaned tests)
|
||||
- **New files without tests**: Log a warning
|
||||
4. Map each changed file to test file(s) using N:M discovery:
|
||||
- Search for `*_test.go` files in the same package directory (not just `<file>_test.go`)
|
||||
- Handle shared test files that cover multiple source files
|
||||
- `internal/foo/bar.go` → `internal/foo/bar_test.go`, `internal/foo/bar_integration_test.go`, etc.
|
||||
- Skip if no matching test files exist (warn user)
|
||||
|
||||
## Phase 3: Test Execution
|
||||
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
|
||||
1. Reuse existing `runTest()` from `internal/cmd/test/cmd_runner.go`
|
||||
- This preserves environment setup (`MACOSX_DEPLOYMENT_TARGET`), output filtering (linker warnings), coverage parsing, JSON support, and consistent styling
|
||||
2. Map smart detection flags to existing `runTest()` parameters:
|
||||
- `--coverage` → `coverage` param (already exists)
|
||||
- `--filter` → `run` param (mapped to `-run`)
|
||||
- Detected test packages → `pkg` param (comma-joined or iterated)
|
||||
3. Do not invoke `go test` directly — all execution goes through `runTest()`
|
||||
|
||||
## 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 ./...`
|
||||
- No changed files → inform user, suggest `--all`
|
||||
- No matching test files → inform user with list of changed files that lack tests
|
||||
- `--all` flag → skip detection, call `runTest()` with `pkg="./..."` (uses existing infrastructure, not raw `go test`)
|
||||
- Mixed renames and edits → deduplicate test file list
|
||||
- Non-Go files changed → skip silently (only `.go` files trigger detection)
|
||||
|
||||
## Files to Create/Modify
|
||||
- `cmd/core/cmd/test.go` (new)
|
||||
- `cmd/core/cmd/root.go` (register command)
|
||||
## Files to Modify
|
||||
- `internal/cmd/test/cmd_main.go` (add `--all`, `--filter`, `--base` flags)
|
||||
- `internal/cmd/test/cmd_runner.go` (add change detection logic before calling existing `runTest()`)
|
||||
- `internal/cmd/test/cmd_detect.go` (new — git diff parsing and file-to-test mapping)
|
||||
|
||||
## Testing
|
||||
- Add `cmd/core/cmd/test_test.go` with unit tests
|
||||
- Add `internal/cmd/test/cmd_detect_test.go` with unit tests for:
|
||||
- File-to-test mapping (1:1, 1:N, renames, deletes)
|
||||
- Git diff parsing (`--name-only`, `--name-status`)
|
||||
- CI vs local context detection
|
||||
- Manual testing with actual git changes
|
||||
|
||||
|
|
|
|||
|
|
@ -1,29 +1,36 @@
|
|||
# Issue 258: Smart Test Detection
|
||||
|
||||
## Original Issue
|
||||
https://github.com/host-uk/core/issues/258
|
||||
<https://github.com/host-uk/core/issues/258>
|
||||
|
||||
## Summary
|
||||
Make `core test` smart - detect changed files and run only relevant tests.
|
||||
Make `core test` smart — detect changed Go files and run only relevant tests.
|
||||
|
||||
> **Scope:** Go-only. The existing `core test` command (`internal/cmd/test/`) targets Go projects (requires `go.mod`). Future language support (PHP, etc.) would be added as separate detection strategies, but this issue covers Go only.
|
||||
|
||||
## Commands
|
||||
```bash
|
||||
core test # Run tests for changed files only
|
||||
core test --all # Run all tests
|
||||
core test --filter UserTest # Run specific test
|
||||
core test --all # Run all tests (skip detection)
|
||||
core test --filter UserTest # Run specific test pattern
|
||||
core test --coverage # With coverage report
|
||||
core test --base origin/dev # Compare against specific base branch (CI)
|
||||
```
|
||||
|
||||
## 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
|
||||
- [ ] Detect changed `.go` files via `git diff` (local: `HEAD`, CI: `origin/dev...HEAD`)
|
||||
- [ ] Handle renames, deletes, and new files via `git diff --name-status`
|
||||
- [ ] Map source files to test files using N:M discovery (`foo.go` → `foo_test.go`, `foo_integration_test.go`, etc.)
|
||||
- [ ] Warn when changed files have no corresponding tests
|
||||
- [ ] Execute tests through existing `runTest()` infrastructure (not raw `go test`)
|
||||
- [ ] Support `--all` flag to skip detection and run all tests
|
||||
- [ ] Support `--filter` flag for test name pattern matching
|
||||
- [ ] Support `--coverage` flag for coverage reports
|
||||
- [ ] Support `--base` flag for CI/PR diff context
|
||||
|
||||
## Technical Context
|
||||
- Go CLI using Cobra
|
||||
- Commands in `cmd/core/cmd/`
|
||||
- Follow existing patterns in `dev_*.go` files
|
||||
|
||||
- Existing `core test` command: `internal/cmd/test/cmd_main.go`
|
||||
- Existing test runner: `internal/cmd/test/cmd_runner.go` (`runTest()`)
|
||||
- Output parsing: `internal/cmd/test/cmd_output.go`
|
||||
- Command registration: `internal/cmd/test/cmd_commands.go` via `cli.RegisterCommands()`
|
||||
- Follow existing patterns in `internal/cmd/test/`
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue