Skip to content

Commit 7d70ef8

Browse files
justin808claude
andcommitted
Add cross-references, success criteria, Playwright testing, and fix formatting
Improvements based on feedback: 1. Quick Reference section - Links to all related documentation at top - Jump-to-section navigation - Makes document easier to navigate 2. Clarified agent invocation - Explains this is documentation, not automated tool - Shows how to use with AI assistants - Provides example prompts 3. Added Playwright E2E Testing section (Section 7) - When E2E tests are required (user-facing changes) - How to run Playwright tests - What to verify in browser - Added to pre-merge checklist 4. Success Criteria section - Defines what "well-tested PR" means - 7 concrete criteria with checkboxes - Example testing documentation format - Clear standards for reviewers and contributors 5. Fixed code block formatting - PR comment template now uses indented code blocks - Avoids nested code block rendering issues - Cleaner markdown output 6. Renumbered sections - Playwright E2E Testing is now section 7 - Rails Engine Changes moved to section 8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 4e9b321 commit 7d70ef8

File tree

1 file changed

+180
-20
lines changed

1 file changed

+180
-20
lines changed

.claude/docs/pr-testing-agent.md

Lines changed: 180 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@
44

55
**Core Principle:** Be deeply suspicious of claims that tests passed unless you have concrete evidence. Assume automated tests have gaps. Manual testing is often required.
66

7+
## Quick Reference
8+
9+
**Related Documentation:**
10+
11+
- [Testing Build Scripts](.claude/docs/testing-build-scripts.md) - Build/package testing requirements
12+
- [CI Config Switching](../SWITCHING_CI_CONFIGS.md) - Testing minimum vs latest dependencies
13+
- [Local Testing Issues](../spec/dummy/TESTING_LOCALLY.md) - Environment-specific testing issues
14+
- [Master Health Monitoring](.claude/docs/master-health-monitoring.md) - Post-merge CI monitoring
15+
- [CLAUDE.md](../CLAUDE.md) - Full development guide with CI debugging
16+
17+
**Jump to Section:**
18+
19+
- [When to Use This Agent](#when-to-use-this-agent)
20+
- [Testing Requirements by Change Type](#testing-requirements-by-change-type)
21+
- [Pre-Merge Testing Checklist](#pre-merge-testing-checklist)
22+
- [Success Criteria](#success-criteria-well-tested-pr)
23+
- [Real-World Scenarios](#real-world-testing-scenarios)
24+
725
## When to Use This Agent
826

927
### Automatic Invocation (Recommended)
@@ -24,17 +42,23 @@
2442
- Unsure what testing is needed for specific changes
2543
- Need a comprehensive testing checklist
2644

27-
### Usage Pattern
45+
### How to Invoke
2846

29-
```bash
30-
# Before creating PR:
31-
# "Use the PR Testing Agent to validate my testing before I create this PR"
47+
**This is a documentation reference, not an automated tool.** Use it by:
48+
49+
1. **Manual reference**: Read this document when creating PRs or reviewing testing
50+
2. **AI assistant prompt**: Ask your AI coding assistant to "follow the PR Testing Agent guidelines" or "validate testing using PR Testing Agent criteria"
51+
3. **Code review checklist**: Reference specific sections during PR reviews
52+
4. **CI failure investigation**: Use the testing checklists when debugging failures
3253

33-
# During PR review:
34-
# "Use the PR Testing Agent to check if this PR has adequate testing coverage"
54+
**Example prompts for AI assistants:**
3555

36-
# When investigating failures:
37-
# "Use the PR Testing Agent to help me debug these CI failures"
56+
```
57+
"Use the PR Testing Agent guidelines to validate my testing before I create this PR"
58+
59+
"Following the PR Testing Agent checklist, what testing is missing for these build config changes?"
60+
61+
"Apply PR Testing Agent criteria: Is this PR ready to merge from a testing perspective?"
3862
```
3963

4064
## Integration with Existing Workflows
@@ -358,7 +382,51 @@ bin/dev
358382
- ✅ bin/dev starts without errors
359383
- ✅ Example component renders in browser
360384

361-
### 7. Rails Engine Changes
385+
### 7. Playwright E2E Testing Changes
386+
387+
**When E2E tests are required:**
388+
389+
Changes affecting user-facing behavior require Playwright E2E verification:
390+
391+
- React component rendering/behavior changes
392+
- Server-side rendering modifications
393+
- React on Rails integration features (component registry, store registry)
394+
- Rails view helpers (`react_component`, `react_component_hash`)
395+
- Generator changes that affect generated code behavior
396+
397+
**Running Playwright tests:**
398+
399+
```bash
400+
cd spec/dummy
401+
402+
# Install browsers (one-time setup):
403+
yarn playwright install --with-deps
404+
405+
# Run all E2E tests (Rails server auto-starts):
406+
yarn test:e2e
407+
408+
# Run in UI mode (interactive debugging):
409+
yarn test:e2e:ui
410+
411+
# Run specific test file:
412+
yarn test:e2e e2e/playwright/e2e/react_on_rails/basic_components.spec.js
413+
```
414+
415+
**What to verify:**
416+
417+
- [ ] Components render in browser
418+
- [ ] Server-side rendering produces correct HTML
419+
- [ ] Client-side hydration works
420+
- [ ] No JavaScript console errors
421+
- [ ] Component interactions work as expected
422+
423+
**See CLAUDE.md "Playwright E2E Testing" section for:**
424+
425+
- Writing new tests with Rails integration
426+
- Using factory_bot and database cleanup
427+
- Debugging test failures
428+
429+
### 8. Rails Engine Changes
362430

363431
**Engine-specific testing:**
364432

@@ -497,6 +565,14 @@ bundle exec rspec spec/system/integration_spec.rb
497565
- [ ] Test in example: `rake run_rspec:example_basic`
498566
- [ ] Test in fresh Rails app (see checklist above)
499567

568+
#### If user-facing behavior changed (React components, SSR, view helpers):
569+
570+
- [ ] Run Playwright E2E tests: `cd spec/dummy && yarn test:e2e`
571+
- [ ] Verify components render in browser
572+
- [ ] Check server-side rendering in view source
573+
- [ ] No JavaScript console errors
574+
- [ ] Component interactions work as expected
575+
500576
#### If Rails engine changed:
501577

502578
- [ ] Check rake tasks not duplicated: `bundle exec rake -T | grep react`
@@ -519,6 +595,91 @@ bundle exec rspec spec/system/integration_spec.rb
519595
**Reviewer**: Please verify these items manually.
520596
```
521597

598+
## Success Criteria: Well-Tested PR
599+
600+
**A PR is well-tested when it meets ALL of these criteria:**
601+
602+
### 1. Automated Testing
603+
604+
**All CI checks pass:**
605+
606+
- RuboCop: 0 violations
607+
- RSpec unit tests: All passing
608+
- Jest tests: All passing
609+
- TypeScript: Compiles without errors
610+
- RBS validation: Types valid
611+
- Prettier: All files formatted
612+
613+
### 2. Local Verification
614+
615+
**Tests run locally before pushing:**
616+
617+
- Relevant test suite executed (unit, integration, or E2E based on changes)
618+
- Test results documented in PR description or commit message
619+
- Any test failures investigated and resolved
620+
621+
### 3. Manual Testing (Change-Dependent)
622+
623+
**Appropriate manual testing completed:**
624+
625+
- **For build changes**: Clean install, prepack, yalc publish tested
626+
- **For UI changes**: Browser testing with visual inspection
627+
- **For SSR changes**: View source to verify HTML output
628+
- **For generator changes**: Tested generation in fresh Rails app
629+
- **For webpack changes**: Debug script created, config verified
630+
631+
### 4. Testing Documentation
632+
633+
**PR includes clear testing documentation:**
634+
635+
```markdown
636+
## Testing
637+
638+
### Automated Tests
639+
640+
- [x] RuboCop: 0 violations
641+
- [x] RSpec: 42 examples, 0 failures
642+
- [x] Jest: 15 tests passed
643+
644+
### Manual Testing
645+
646+
- [x] Tested in browser: Components render correctly
647+
- [x] Server-side rendering verified in view source
648+
- [x] No console errors
649+
650+
### Environment Limitations
651+
652+
- [ ] Integration tests require full Rails app (not available in Conductor)
653+
Documented commands for reviewer verification below
654+
```
655+
656+
### 5. Clear Communication
657+
658+
**Testing status is explicit and honest:**
659+
660+
- Uses "UNTESTED" label when verification not possible
661+
- Distinguishes verified fixes from hypothetical fixes
662+
- Documents environmental limitations clearly
663+
- Provides exact reproduction steps for reviewers
664+
665+
### 6. No Regressions
666+
667+
**Changes don't break existing functionality:**
668+
669+
- Existing tests still pass
670+
- No new console errors introduced
671+
- Build artifacts still generated correctly
672+
- No performance degradation (if measurable)
673+
674+
### 7. CI Failure Investigation
675+
676+
**If CI fails, proper investigation done:**
677+
678+
- Checked if failures pre-existed on master
679+
- Reproduced failures locally (or documented why not possible)
680+
- Identified root cause (not just "fixed randomly")
681+
- Verified fix locally before pushing
682+
522683
## Communicating Test Status
523684

524685
### In PR Comments
@@ -537,17 +698,16 @@ bundle exec rspec spec/system/integration_spec.rb
537698

538699
### ⚠️ Requires Manual Verification
539700

540-
- [ ] **Build scripts** - CRITICAL: Clean install test in full environment
541-
`bash
542-
rm -rf node_modules && yarn install --frozen-lockfile
543-
yarn run yalc:publish
544-
`
545-
- [ ] **Browser testing** - Dummy app visual inspection
546-
`bash
547-
cd spec/dummy && bin/dev
548-
# Visit http://localhost:3000/hello_world
549-
# Check console for errors
550-
`
701+
**Build scripts** - CRITICAL: Clean install test in full environment
702+
703+
rm -rf node_modules && yarn install --frozen-lockfile
704+
yarn run yalc:publish
705+
706+
**Browser testing** - Dummy app visual inspection
707+
708+
cd spec/dummy && bin/dev
709+
# Visit http://localhost:3000/hello_world
710+
# Check console for errors
551711

552712
### ❌ Cannot Test (Environment Limitation)
553713

0 commit comments

Comments
 (0)