From 8006eb782f49950d2c0e20374da38e92be98fc62 Mon Sep 17 00:00:00 2001 From: sheiksyedm Date: Mon, 23 Feb 2026 15:47:43 +0530 Subject: [PATCH 1/2] PR finalize: Add test-only PR handling - Skip Note block recommendation for test-only PRs - Recommend [Testing] prefix for test-only PR titles - Add detection criteria: [Testing] tag, area-testing label, or test files only Test-only PRs don't need the Note block since users don't need to test artifacts for test infrastructure changes. Instead, they should have [Testing] prefix in the title for easy identification. Detection criteria: - Title contains [Testing] tag - PR has area-testing label - PR only adds/modifies test files without functional code - Description explicitly states it's for test coverage only Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 4 +- .github/skills/pr-finalize/SKILL.md | 62 ++++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 5c9f96eba10f..d162f4d6b77e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -168,7 +168,7 @@ git push ### Opening PRs -All PRs are required to have this at the top of the description: +**Issue-fix PRs** (PRs that fix bugs or add functional features) are required to have this at the top of the description: ``` @@ -179,6 +179,8 @@ All PRs are required to have this at the top of the description: Always put that at the top, without the block quotes. Without it, users will NOT be able to try the PR and your work will have been in vain! +**Test-only PRs** (PRs with `[Testing]` tag, `area-testing` label, or only test file changes) do NOT need the NOTE block, as users don't need to test artifacts for test infrastructure changes. + ## Custom Agents and Skills diff --git a/.github/skills/pr-finalize/SKILL.md b/.github/skills/pr-finalize/SKILL.md index c1c8af7144de..32337b4b71aa 100644 --- a/.github/skills/pr-finalize/SKILL.md +++ b/.github/skills/pr-finalize/SKILL.md @@ -108,6 +108,7 @@ Ask: "Is the existing description better than what my template would produce?" | Requirement | Good | Bad | |-------------|------|-----| | Platform prefix (if specific) | `[iOS] Fix Shell back button` | `Fix Shell back button` | +| **Test-only prefix** | `[Testing] Add CollectionView scroll tests` | `Add CollectionView scroll tests` (if test-only) | | Describes behavior, not issue | `[iOS] SafeArea: Return Empty for non-ISafeAreaView views` | `Fix #23892` | | Captures the "what" | `Return Empty for non-ISafeAreaView` | `Fix SafeArea bug` | | Notes model change if applicable | `(opt-in model)` | (omitted) | @@ -115,22 +116,48 @@ Ask: "Is the existing description better than what my template would produce?" ### Title Formula +**For issue-fix PRs:** ``` [Platform] Component: What changed (model change if any) ``` +**For test-only PRs:** +``` +[Testing] Component: What test coverage added +``` + Examples: - `[iOS] SafeArea: Return Empty for non-ISafeAreaView views (opt-in model)` - `[Android] CollectionView: Fix scroll position reset on item update` - `[Windows] Shell: Use NavigationView instead of custom flyout` +- `[Testing] CollectionView: Add scrolling feature matrix test cases` +- `[Testing] Add UI tests for Button click events` ## Description Requirements PR description should: -1. Start with the required NOTE block (so users can test PR artifacts) +1. **For issue-fix PRs**: Start with the required NOTE block (so users can test PR artifacts). **Skip for test-only PRs** (e.g., `[Testing]` tag in title, PR only adds tests without functional fixes). 2. Include the base sections from `.github/PULL_REQUEST_TEMPLATE.md` ("Description of Change" and "Issues Fixed"). The skill adds additional structured fields (Root cause, Fix, Key insight, etc.) as recommended enhancements for better agent context. 3. Match the actual implementation +### Detecting Test-Only PRs + +A PR is considered "test-only" if: +- Title contains `[Testing]` tag +- PR has `area-testing` label +- PR only adds or modifies test files without functional code changes +- PR description explicitly states it's for test coverage only + +**For test-only PRs:** +- Do NOT recommend adding the NOTE block (users don't need to test artifacts for test infrastructure changes) +- **DO recommend adding `[Testing]` prefix to title if missing** (helps identify test-only PRs at a glance) + +**For issue-fix PRs:** +- Always recommend the NOTE block +- Use platform prefix like `[iOS]`, `[Android]`, `[Windows]` if platform-specific + +**For issue-fix PRs:** Always recommend the NOTE block: + ```markdown > [!NOTE] @@ -196,7 +223,7 @@ Example: "Before: Safe area applied by default (opt-out). After: Only views impl | Description doesn't match code | Implementation changed during review | Update description to match actual diff | | Missing root cause | Author focused on "what" not "why" | Add root cause from issue/analysis | | References wrong approach | Started with A, switched to B | Update to describe final approach | -| Missing NOTE block | Author didn't use template | Prepend NOTE block, keep rest | +| Missing NOTE block | Author didn't use template | Prepend NOTE block (unless test-only PR) | | Good description replaced | Agent used template blindly | Evaluate existing quality first | ## Output Format @@ -208,7 +235,8 @@ Example: "Before: Safe area applied by default (opt-out). After: Only views impl ### ✅ Title: [Good / Needs Update] **Current:** "Existing title" -**Recommended:** "[Platform] Improved title" (if needed) +**Recommended:** "[Platform] Improved title" (if issue-fix PR) +**Recommended:** "[Testing] Improved title" (if test-only PR) ### ✅ Description: Excellent - Keep As-Is @@ -218,10 +246,32 @@ Example: "Before: Safe area applied by default (opt-out). After: Only views impl - Accuracy: ✅ Matches implementation - Completeness: ✅ Platforms, breaking changes noted -**Only addition needed:** -- ❌ Missing NOTE block - prepend to top +**Only addition needed (if issue-fix PR):** +- ❌ Missing NOTE block - prepend to top (skip for test-only PRs) + +**Action:** Add NOTE block (unless test-only PR), preserve everything else. +``` + +### Test-Only PR Example + +```markdown +## PR #33632 Finalization Review + +### ⚠️ Title: Needs Update +**Current:** "Additional Feature Matrix Test Cases for CollectionView - 2" +**Recommended:** "[Testing] Additional Feature Matrix Test Cases for CollectionView - 2" + +**Issues:** +- Missing `[Testing]` prefix for test-only PR + +### ✅ Description: Good - No Changes Needed + +**Quality assessment:** +- Structure: ✅ Clear sections +- Test coverage: ✅ Well documented +- Accuracy: ✅ Matches implementation -**Action:** Add NOTE block, preserve everything else. +**No NOTE block needed** - This is a test-only PR (has `area-testing` label) ``` ### When Description Needs Rewrite From 780e9d7785263e328c6a5dc47b24a8b9c8ddff1f Mon Sep 17 00:00:00 2001 From: sheiksyedm Date: Mon, 23 Feb 2026 18:01:37 +0530 Subject: [PATCH 2/2] Address PR review comments: improve test-only wording and remove duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolves copilot review feedback on PR #34185: 1. Improved test-only rationale (SKILL.md line 152) - Changed 'test infrastructure changes' to broader wording - Now covers all test changes, not just infrastructure - Clarifies users don't need to test artifacts for test-only changes 2. Removed duplicate 'For issue-fix PRs' heading (SKILL.md line 159) - Was listed twice in a row (bullets + heading) - Consolidated to single clear heading - Improved readability and reduced redundancy 3. Consistent terminology (copilot-instructions.md line 182) - Changed 'test infrastructure changes' → 'test-only changes' - Matches broader criteria (any test changes, not just infra) 4. Scoped 'Always put that at the top' (copilot-instructions.md line 180) - Now explicitly states: 'For issue-fix PRs, always put that at the top' - Removes ambiguity with test-only PRs (which skip NOTE block) All changes maintain consistency with test-only PR detection criteria and clarify that the NOTE block exception applies to all test changes. --- .github/copilot-instructions.md | 4 ++-- .github/skills/pr-finalize/SKILL.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d162f4d6b77e..e06aae78d588 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -177,9 +177,9 @@ git push > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ``` -Always put that at the top, without the block quotes. Without it, users will NOT be able to try the PR and your work will have been in vain! +For issue-fix PRs, always put that at the top, without the block quotes. Without it, users will NOT be able to try the PR and your work will have been in vain! -**Test-only PRs** (PRs with `[Testing]` tag, `area-testing` label, or only test file changes) do NOT need the NOTE block, as users don't need to test artifacts for test infrastructure changes. +**Test-only PRs** (PRs with `[Testing]` tag, `area-testing` label, or only test file changes) do NOT need the NOTE block, as users don't need to test artifacts for test-only changes. diff --git a/.github/skills/pr-finalize/SKILL.md b/.github/skills/pr-finalize/SKILL.md index 32337b4b71aa..e01b9cdded1b 100644 --- a/.github/skills/pr-finalize/SKILL.md +++ b/.github/skills/pr-finalize/SKILL.md @@ -149,14 +149,14 @@ A PR is considered "test-only" if: - PR description explicitly states it's for test coverage only **For test-only PRs:** -- Do NOT recommend adding the NOTE block (users don't need to test artifacts for test infrastructure changes) +- Do NOT recommend adding the NOTE block (users generally don't need to manually test PR artifacts when changes only affect automated tests or test infrastructure and have no user-facing behavior impact) - **DO recommend adding `[Testing]` prefix to title if missing** (helps identify test-only PRs at a glance) **For issue-fix PRs:** - Always recommend the NOTE block - Use platform prefix like `[iOS]`, `[Android]`, `[Windows]` if platform-specific -**For issue-fix PRs:** Always recommend the NOTE block: +Use this NOTE block template in the description: ```markdown