Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/README-AI.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,11 @@ This is by design - it's better to pause and get guidance than provide incomplet
### Instruction Files
These files provide specialized guidance for specific scenarios:

- **`instructions/common-testing-patterns.md`** - Common testing patterns for command sequences (UDID extraction, builds, deploys, error checking)
- **`instructions/uitests.instructions.md`** - UI testing guidelines (when to use HostApp vs Sandbox)
- **`instructions/safearea-testing.instructions.md`** - SafeArea testing patterns (measure children, not parents)
- **`instructions/instrumentation.instructions.md`** - Code instrumentation patterns for debugging and testing
- **`instructions/appium-control.instructions.md`** - Standalone Appium scripts for manual debugging and exploration
- **`instructions/templates.instructions.md`** - Template modification rules and conventions

### General Guidelines
Expand Down
98 changes: 87 additions & 11 deletions .github/agents/pr-reviewer.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ You are a specialized PR review agent for the .NET MAUI repository. Your role is

1. **Read Required Files**:
- `.github/copilot-instructions.md` - General coding standards
- `.github/instructions/common-testing-patterns.md` - Command patterns with error checking
- `.github/instructions/instrumentation.instructions.md` - Testing patterns
- `.github/instructions/safearea-testing.instructions.md` - If SafeArea-related PR
- `.github/instructions/uitests.instructions.md` - If PR adds/modifies UI tests
Expand Down Expand Up @@ -178,7 +179,15 @@ When testing is required, use the Sandbox app to validate PR changes:

### Fetch PR Changes (Without Checking Out)

**CRITICAL**: Stay on the current branch (pr-reviewer) to preserve all instruction files and context. Apply PR changes on top of the current branch instead of checking out the PR branch.
**CRITICAL**: Stay on your current branch (wherever you are when starting the review) to preserve context. Apply PR changes on top of the current branch instead of checking out the PR branch.

**FIRST STEP - Record Your Starting Branch:**
```bash
# Record what branch you're currently on - you'll need this for cleanup
ORIGINAL_BRANCH=$(git branch --show-current)
echo "Starting review from branch: $ORIGINAL_BRANCH"
# Remember this value for cleanup at the end!
```

```bash
# Get the PR number from the user's request
Expand All @@ -187,11 +196,30 @@ PR_NUMBER=XXXXX # Replace with actual PR number
# Fetch the PR into a temporary branch
git fetch origin pull/$PR_NUMBER/head:pr-$PR_NUMBER-temp

# Check fetch succeeded
if [ $? -ne 0 ]; then
echo "❌ ERROR: Failed to fetch PR #$PR_NUMBER"
exit 1
fi

# Create a test branch from current branch (preserves instruction files)
git checkout -b test-pr-$PR_NUMBER

# Check branch creation succeeded
if [ $? -ne 0 ]; then
echo "❌ ERROR: Failed to create test branch"
exit 1
fi

# Merge the PR changes into the test branch
git merge pr-$PR_NUMBER-temp -m "Test PR #$PR_NUMBER" --no-edit

# Check merge succeeded (will error if conflicts)
if [ $? -ne 0 ]; then
echo "❌ ERROR: Merge failed with conflicts"
echo "See section below on handling merge conflicts"
exit 1
fi
```

**If merge conflicts occur:**
Expand Down Expand Up @@ -248,28 +276,47 @@ I need help resolving this merge issue before I can test the PR.
**Why this matters**: If you can't cleanly merge the PR, you can't accurately test it. Testing with incorrect code leads to misleading results. It's better to pause and get help than to provide an incomplete or incorrect review.

**Why this approach:**
- βœ… Preserves all instruction files from pr-reviewer branch
- βœ… Tests PR changes on top of latest guidelines
- βœ… Simple and reliable (standard git merge)
- βœ… Easy to clean up (just delete test branch)
- βœ… Preserves your current working context and branch state
- βœ… Tests PR changes on top of wherever you currently are
- βœ… Allows agent to maintain proper context across review
- βœ… Easy to clean up (just delete test branch and return to original branch)
- βœ… Can compare before/after easily
- βœ… Handles most conflicts gracefully

### Setup Test Environment

**iOS Testing**:
```bash
# Find iOS 26 simulator (or specify version based on issue)
UDID=$(xcrun simctl list devices available --json | jq -r '.devices["com.apple.CoreSimulator.SimRuntime.iOS-26-0"] | first | .udid')
# Find iPhone Xs with highest iOS version
UDID=$(xcrun simctl list devices available --json | jq -r '.devices | to_entries | map(select(.key | startswith("com.apple.CoreSimulator.SimRuntime.iOS"))) | map({key: .key, version: (.key | sub("com.apple.CoreSimulator.SimRuntime.iOS-"; "") | split("-") | map(tonumber)), devices: .value}) | sort_by(.version) | reverse | map(select(.devices | any(.name == "iPhone Xs"))) | first | .devices[] | select(.name == "iPhone Xs") | .udid')

# Check UDID was found
if [ -z "$UDID" ] || [ "$UDID" = "null" ]; then
echo "❌ ERROR: No iPhone Xs simulator found. Please create one."
exit 1
fi

# Boot simulator
xcrun simctl boot $UDID 2>/dev/null || true

# Check simulator is booted
STATE=$(xcrun simctl list devices --json | jq -r --arg udid "$UDID" '.devices[][] | select(.udid == $udid) | .state')
if [ "$STATE" != "Booted" ]; then
echo "❌ ERROR: Simulator failed to boot. Current state: $STATE"
exit 1
fi
```

**Android Testing**:
```bash
# Get connected device/emulator
export DEVICE_UDID=$(adb devices | grep -v "List" | grep "device" | awk '{print $1}' | head -1)

# Check device was found
if [ -z "$DEVICE_UDID" ]; then
echo "❌ ERROR: No Android device/emulator found. Start an emulator or connect a device."
exit 1
fi
```

### Modify Sandbox App for Testing
Expand Down Expand Up @@ -382,11 +429,30 @@ Does this test approach look correct before I build and deploy?
# Build
dotnet build src/Controls/samples/Controls.Sample.Sandbox/Maui.Controls.Sample.Sandbox.csproj -f net10.0-ios

# Check build succeeded
if [ $? -ne 0 ]; then
echo "❌ ERROR: Build failed"
exit 1
fi

# Install
xcrun simctl install $UDID artifacts/bin/Maui.Controls.Sample.Sandbox/Debug/net10.0-ios/iossimulator-arm64/Maui.Controls.Sample.Sandbox.app

# Check install succeeded
if [ $? -ne 0 ]; then
echo "❌ ERROR: App installation failed"
exit 1
fi

# Launch with console capture
xcrun simctl launch --console-pty $UDID com.microsoft.maui.sandbox > /tmp/ios_test.log 2>&1 &

# Check launch didn't immediately fail
if [ $? -ne 0 ]; then
echo "❌ ERROR: App launch failed"
exit 1
fi

sleep 8
cat /tmp/ios_test.log
```
Expand All @@ -396,6 +462,12 @@ cat /tmp/ios_test.log
# Build and deploy
dotnet build src/Controls/samples/Controls.Sample.Sandbox/Maui.Controls.Sample.Sandbox.csproj -f net10.0-android -t:Run

# Check build/deploy succeeded
if [ $? -ne 0 ]; then
echo "❌ ERROR: Build or deployment failed"
exit 1
fi

# Monitor logs
adb logcat | grep -E "(YourMarker|Frame|Console)"
```
Expand Down Expand Up @@ -614,12 +686,14 @@ See `.github/instructions/safearea-testing.instructions.md` for comprehensive gu

6. **Clean up test branches**
```bash
# Return to pr-reviewer branch
git checkout pr-reviewer
# Return to original branch (whatever branch you started on)
git checkout $ORIGINAL_BRANCH

# Delete test branches
git branch -D test-pr-XXXXX baseline-test pr-XXXXX-temp
```

**Note**: Uses `$ORIGINAL_BRANCH` variable you set at the beginning. If you didn't save it, replace with whatever branch you were on when you started the review (e.g., `main`, `pr-reviewer`, etc.)

### Include Test Results in Review

Expand Down Expand Up @@ -682,8 +756,8 @@ For each edge case tested, document:
After testing, clean up all test artifacts:

```bash
# Return to pr-reviewer branch
git checkout pr-reviewer
# Return to your original branch (use the variable from the beginning)
git checkout $ORIGINAL_BRANCH # Or manually specify: main, pr-reviewer, etc.

# Revert any changes to Sandbox app
git checkout -- src/Controls/samples/Controls.Sample.Sandbox/
Expand All @@ -695,6 +769,8 @@ git branch -D test-pr-XXXXX baseline-test pr-XXXXX-temp 2>/dev/null || true
dotnet clean
```

**Important**: If you didn't save `$ORIGINAL_BRANCH` at the start, replace it with whatever branch you were on when you began the review. This ensures you return to your starting state.

## Core Responsibilities

1. **Code Quality Review**: Analyze code for correctness, performance, maintainability, and adherence to .NET MAUI coding standards
Expand Down
32 changes: 21 additions & 11 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ When updating this file, you MUST also update `AGENTS.md` (in repository root) t
- **.NET SDK** - Version is **ALWAYS** defined in `global.json` at repository root
- **main branch**: Use the latest stable version of .NET to build (currently .NET 10)
- **net10.0 branch**: Use the latest .NET 10 SDK
- **etc.**: Each feature branch correlates to its respective .NET version
- **Feature branches**: Each feature branch (e.g., `net11.0`, `net12.0`) correlates to its respective .NET version
- **C#** and **XAML** for application development
- **Cake build system** for compilation and packaging
- **MSBuild** with custom build tasks
Expand Down Expand Up @@ -91,10 +91,18 @@ dotnet build ./Microsoft.Maui.BuildTasks.slnf --verbosity normal
- **Windows** specific code is inside folders named `Windows`

### Platform-Specific File Extensions
- Files with `.windows.cs` will only compile for the Windows TFM
- Files with `.android.cs` will only compile for the Android TFM
- Files with `.ios.cs` will only compile for the iOS and MacCatalyst TFM
- Files with `MacCatalyst.cs` will only compile for the MacCatalyst TFM

Platform-specific files use naming conventions to control compilation:

**File extension patterns**:
- `.windows.cs` - Windows TFM only
- `.android.cs` - Android TFM only
- `.ios.cs` - iOS and MacCatalyst TFMs (both)
- `.maccatalyst.cs` - MacCatalyst TFM only (does NOT compile for iOS)

**Important**: Both `.ios.cs` and `.maccatalyst.cs` files compile for MacCatalyst. There is no precedence mechanism that excludes one when the other exists.

**Example**: If you have both `CollectionView.ios.cs` and `CollectionView.maccatalyst.cs`, both will compile for MacCatalyst builds. The `.maccatalyst.cs` file won't compile for iOS, but the `.ios.cs` file will compile for both iOS and MacCatalyst.

### Sample Projects
```
Expand Down Expand Up @@ -133,12 +141,12 @@ dotnet cake --target=dotnet-pack

#### Testing Guidelines
- Add tests for new functionality
- Ensure existing tests pass:
- `src/Core/tests/UnitTests/Core.UnitTests.csproj`
- `src/Essentials/test/UnitTests/Essentials.UnitTests.csproj`
- `src/Compatibility/Core/tests/Compatibility.UnitTests/Compatibility.Core.UnitTests.csproj`
- `src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj`
- `src/Controls/tests/Xaml.UnitTests/Controls.Xaml.UnitTests.csproj`
- Ensure existing tests pass for modified areas (major test projects):
- **Core tests**: `src/Core/tests/UnitTests/Core.UnitTests.csproj`
- **Essentials tests**: `src/Essentials/test/UnitTests/Essentials.UnitTests.csproj`
- **Controls tests**: `src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj`
- **XAML tests**: `src/Controls/tests/Xaml.UnitTests/Controls.Xaml.UnitTests.csproj`
- **To find other test projects**: `find . -name "*.UnitTests.csproj"` or check the solution file

#### UI Testing

Expand Down Expand Up @@ -305,11 +313,13 @@ dotnet format analyzers Microsoft.Maui.slnx

## Additional Resources

- [Common Testing Patterns](/.github/instructions/common-testing-patterns.md) - Common command patterns for UDID extraction, builds, deploys, and error checking
- [UI Testing Guide](../docs/UITesting-Guide.md)
- [UI Testing Architecture](../docs/design/UITesting-Architecture.md)
- [PR Test Validation Guide](../docs/PR-Test-Validation-Guide.md) - Procedures for validating UI tests in PRs
- [SafeArea Testing Guide](/.github/instructions/safearea-testing.instructions.md) - Specialized guide for testing SafeArea changes (measure children, not parents)
- [Instrumentation Guide](/.github/instructions/instrumentation.instructions.md) - Patterns for instrumenting MAUI code for debugging and testing
- [Appium Control Scripts](/.github/instructions/appium-control.instructions.md) - Create standalone scripts for manual Appium-based debugging and exploration
- [Development Guide](/.github/DEVELOPMENT.md)
- [Development Tips](/docs/DevelopmentTips.md)
- [Contributing Guidelines](/.github/CONTRIBUTING.md)
Expand Down
Loading
Loading