Test decision guide

[kubaflo]

✅ Complete - Instructions Improved

I've created two comprehensive documents to prevent this issue in future sessions:

1. Test Type Decision Guide (.github/instructions/test-type-decision-guide.md)

Contents:

 - ✅ Quick Decision Flowchart - Visual guide for test type selection
 - ✅ Decision Matrix - Table mapping scenarios to test types
 - ✅ Detailed Decision Rules - When to use unit vs UI tests
 - ✅ How to Recognize from PR Changes - Clues from modified files
 - ✅ Case Study: PR #32939 - Your exact scenario analyzed as an example
 - ✅ Updated Decision Process - Step-by-step guide with bash commands

Key Rules Added:

 Rule #1: Follow the PR Author's Lead
   If PR includes unit tests → Add more unit tests
   If PR includes UI tests → Add more UI tests
   If PR includes no tests → Analyze and decide
 
 Rule #2: Prefer Unit Tests When Possible
   If behavior can be tested without UI → Unit tests
   Only use UI tests when visual/interactive verification is required
 
 Rule #3: Sandbox is for Exploration
   Use Sandbox for manual exploratory testing, not as a replacement for formal tests

2. Lessons Learned Document (CustomAgentLogsTmp/Lessons-Learned-PR32939.md)

Contents:

 - ✅ What I did and what I should have done
 - ✅ Why Sandbox approach was suboptimal for PR #32939
 - ✅ Red flags I missed that indicated unit tests were correct
 - ✅ What the correct approach would have been (10 mins vs 45 mins)
 - ✅ Positive outcomes (review was still valuable)

---

Key Improvements for Future Sessions

What Will Change:

Before (What I Did):

 - See PR request
 - Default to Sandbox + Appium testing
 - Create UI scenario
 - Deploy to device
 - Run visual tests

After (What I'll Do):

 - See PR request
 - Check if PR includes tests ← NEW STEP
 - If unit tests exist → Run them, validate coverage
 - If UI tests exist → Review and run them
 - If no tests → Analyze what's being changed:
   - Control properties/logic? → Unit tests
   - Visual/layout/handlers? → UI tests
   - Sandbox only for manual exploration

Decision Signals I'll Watch For:

✅ Unit Tests Indicated When:

 - PR diff shows Core.UnitTests/[Control]Tests.cs changes
 - Only control source files modified (no handlers)
 - PR description focuses on "property", "value", "event", "order"
 - Changes to BindableProperty definitions
 - No visual/layout keywords in description

✅ UI Tests Indicated When:

 - PR diff shows platform handler files (.Android.cs, .iOS.cs)
 - PR diff shows layout/measurement code
 - PR description mentions "rendering", "visual", "layout", "padding"
 - PR description mentions "tap", "gesture", "navigation"
 - Changes affect visual appearance or interactions

---

For This Specific Case (PR dotnet#32939)

What I should have done:

 # 1. Notice PR has unit tests
 gh pr diff 32939 --name-only | grep UnitTests
 
 # 2. Run existing tests
 dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj \
   --filter "FullyQualifiedName~Slider|Stepper"
 
 # 3. Verify all pass
 # ✅ 98 tests pass
 
 # 4. Write review focusing on code quality and test coverage

Time saved: 35 minutes (Sandbox build/deploy/test avoided)

---

Your Feedback Applied

Your observation was spot-on: PR dotnet#32939 should have been validated with unit tests, not UI tests in Sandbox.

The new decision guide will prevent this in future sessions by:

 - Always checking if PR includes tests first
 - Following the PR author's test approach
 - Choosing unit tests for property logic (no UI needed)
 - Reserving Sandbox for manual exploration, not formal validation

Thank you for the feedback! The instructions are now significantly improved. 🙏