Skip to content

Commit 4ca2dc9

Browse files
Add PR manual test verification process (br#21) (#29)
Add PR template, docs page, sidebar/index links, and GitHub labels to require manual test verification on PRs that change CLI behavior, skill management, or platform adapters. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a5d1d51 commit 4ca2dc9

4 files changed

Lines changed: 88 additions & 0 deletions

File tree

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
## Summary
2+
<!-- Brief description of what this PR does and why. -->
3+
4+
## Changes
5+
<!-- Key changes, organized by area if needed. -->
6+
7+
## Testing
8+
- [ ] Unit tests pass (`make test`)
9+
- [ ] Lint passes (`make lint`)
10+
- [ ] Smoke tests pass (`make test-smoke`) *(if applicable)*
11+
12+
## Manual Test Verification
13+
<!--
14+
For PRs that change CLI behavior, skill management, or platform adapters:
15+
a maintainer must post a manual test verification comment before merge.
16+
17+
Format: https://skern.dev/contributing/pr-manual-testing
18+
19+
Exempt: docs-only, CI-config-only, or trivially mechanical PRs.
20+
-->
21+
- [ ] Manual test verification comment posted (or PR is exempt)

docs/.vitepress/config.mts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export default defineConfig({
8383
{ text: 'Contributing Guide', link: '/contributing/' },
8484
{ text: 'Development', link: '/contributing/development' },
8585
{ text: 'Manual Testing', link: '/contributing/manual-testing' },
86+
{ text: 'PR Manual Testing', link: '/contributing/pr-manual-testing' },
8687
],
8788
},
8889
],

docs/contributing/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ cd skern
2727

2828
- [Development](/contributing/development) — build, test, and lint commands
2929
- [Manual Testing](/contributing/manual-testing) — agent test harness for pre-release testing
30+
- [PR Manual Testing](/contributing/pr-manual-testing) — manual test verification for pull requests
3031

3132
## Issue Tracking
3233

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# PR Manual Testing
2+
3+
Some skern changes — CLI behavior, skill management, platform adapters, registry logic — require manual verification that CI cannot provide. This page documents the lightweight manual test verification step required on PRs before merge.
4+
5+
## When Is It Required?
6+
7+
Manual test verification is required for PRs that touch:
8+
9+
- CLI commands or flags (`internal/cli/`)
10+
- Skill management logic (`internal/skill/`, `internal/registry/`)
11+
- Platform adapters (`internal/platform/`)
12+
- Overlap detection (`internal/overlap/`)
13+
- Output formatting (`internal/output/`)
14+
15+
### Exemptions
16+
17+
The following PRs are exempt from manual test verification:
18+
19+
- Documentation-only changes (`docs/`, `*.md` outside `internal/`)
20+
- CI/CD configuration changes (`.github/workflows/`)
21+
- Trivially mechanical changes (typo fixes, import reordering, comment edits)
22+
23+
Mark exempt PRs with a comment noting the exemption reason, then check the template checkbox.
24+
25+
## Comment Format
26+
27+
A maintainer posts a PR comment with the following table before merge:
28+
29+
````markdown
30+
### Manual Test Verification
31+
32+
| # | Scenario | Result | Notes |
33+
|---|----------|--------|-------|
34+
| 1 | `skern skill list` shows expected skills | :white_check_mark: | |
35+
| 2 | `skern skill install <name> --platform claude-code` works | :white_check_mark: | |
36+
| 3 | `skern platform status` reflects installed skill | :white_check_mark: | |
37+
| 4 | *<add rows relevant to the PR>* | | |
38+
39+
**Tested at:** `<commit SHA>`
40+
**Platform:** `<OS / shell>`
41+
````
42+
43+
Adapt the rows to the specific PR — only test what the PR changes. Keep it brief.
44+
45+
## Rules
46+
47+
1. **Pass / Fail / Skip** — use :white_check_mark: (pass), :x: (fail), or :fast_forward: (skip with reason in Notes).
48+
2. **All rows must pass** before merge. If any row fails, the PR must be updated and re-tested.
49+
3. **Pin the commit SHA** — record which commit was tested. If new commits are pushed after the verification comment, the test must be re-run.
50+
4. **Brevity** — test only what the PR changes. Don't re-run the full agent test harness (that's a separate concern, see below).
51+
52+
## Labels
53+
54+
Two GitHub labels track manual test status:
55+
56+
| Label | Color | Meaning |
57+
|-------|-------|---------|
58+
| `needs-manual-test` | yellow | PR requires manual test verification before merge |
59+
| `manual-test-verified` | green | Manual test verification has been posted and passes |
60+
61+
Maintainers apply `needs-manual-test` when opening or reviewing a non-exempt PR, and replace it with `manual-test-verified` after a passing verification comment.
62+
63+
## Relationship to the Agent Test Harness
64+
65+
The [agent test harness](/contributing/manual-testing) (`tests/manual/`) is a separate, more comprehensive concern — it covers 10 end-to-end scenarios for pre-release validation. PR manual testing is lighter: it verifies only the specific behavior changed by the PR. The two complement each other but are independent processes.

0 commit comments

Comments
 (0)