ci: Run tvOS on PRs only for Apple changes by MartinZikmund · Pull Request #24950 · unoplatform/uno

MartinZikmund · 2026-10-02T16:34:32Z

GitHub Issue: closes #24949

PR Type:

🏗️ Build or CI related changes

What changed? 🚀

Tests - tvOS Skia now runs on a PR only when determine-test-scope.ps1 finds Apple-specific changes. A new TvOSTestsRequired scope matches any path segment or file-name part tvos, ios, uikit, appleuikit or apple: *.iOS.cs, *.UIKit.cs, *.Apple.cs, Uno.UI.Runtime.Skia.AppleUIKit/, the tvOS/iOS CI templates and test scripts. Words like scenarios or ratios don't match.

Post-merge builds (master, release, feature) still run tvOS every time. If the scope script can't fetch the target branch, it enables every scope, tvOS included.

Why: iOS and tvOS together queue 10 shards on the small Mac pool. In clean PR builds since #24638, iOS shards waited a median 29 min for an agent and tvOS 33 min, longer than either actually ran. iOS Skia still runs on every PR and covers the shared Skia layer.

This PR touches no Apple paths, so its own CI run should show tvOS skipped and the iOS shards waiting less.

PR Checklist ✅

🧪 Added Runtime tests, UI tests, or a manual test sample (n/a, CI-only)
📚 Docs have been added/updated (n/a)
🖼️ Validated PR Screenshots Compare Test Run results.
❗ Contains NO breaking changes
👀 Reviewed 2 other open pull requests

🤖 Generated with Claude Code

https://claude.ai/code/session_012psuk5dxG4cNcryRVL2VWS

github-actions · 2026-10-02T16:34:54Z

Claude finished @MartinZikmund's task in 2m 52s —— View job

Review complete ✅

Gather context (PR diff, AGENTS.md rules)
Review determine-test-scope.ps1 changes
Review .azure-devops-tests-skia-stages.yml changes
Post inline findings
Post summary

Verdict: Ship (1 non-blocking observation)

Summary

This is a small, well-scoped CI change: it adds a TvOSTestsRequired scope variable alongside the existing TemplateTestsRequired/ScreenshotsRequired pattern, and gates the runtime_tests_skia_tvos stage behind it using the exact same condition: idiom (and(succeeded(), or(ne(variables['Build.Reason'], 'PullRequest'), eq(stageDependencies.Setup...), 'true')))) already proven on the template-tests stage — same dependsOn: Setup dependency shape, so succeeded() (vs. succeededOrFailed()) is the correct choice here, matching precedent rather than the screenshots stage's variant.

I traced the new regex ((?:^|[/._-])(?:tvos|ios|uikit|appleuikit|apple)(?:[/._-]|$)) against a number of adversarial path/filename cases (scenarios.cs, ratios, radios, portfolios, pineapple.cs, bios.cfg, etc.) — the boundary-character requirement on both sides correctly avoids false positives on all of them, and correctly matches real suffixes/folders in the repo (.UIKit.cs, src/Uno.UI.Runtime.Skia.AppleUIKit/, *.tvOS.targets). The non-PR and git-fetch-failure fallback paths (enable-all-scopes) are untouched and still apply to the new variable for free since it's just another key in the same ordered hashtable.

One non-blocking design observation posted inline: the new scope is purely path/keyword-based, so it doesn't match edits to the shared build/ci/tests/.azure-devops-tests-skia-stages.yml file that contains the tvOS stage's own gating condition — meaning future changes to that gating logic (like this PR) can't be self-validated by the lane they gate. Worth a look, not a blocker.

No source/test code, DependencyProperty, XAML, or event-pattern surfaces touched by this PR, so the rest of the AGENTS.md checklist (nullable/style, async discipline, platform suffixes, etc.) doesn't apply here.

Copilot

Copilot review overview

🟡 Changes recommended

The matcher misses Apple resource tasks used by tvOS builds.

Review effort: Balanced
Findings: 1

Open (1)

Include shared iOS resource tasks in tvOS path matching · New

What changed in this PR

Limits tvOS CI on pull requests to Apple-specific changes while preserving post-merge and fallback coverage.

Changes:

Adds a tvOS test-scope variable and path matcher.
Conditionally runs the tvOS stage on PRs.
Review found one scope-matching gap.

File	Description
`build/ci/tests/.azure-devops-tests-skia-stages.yml`	Gates the tvOS stage by scope.
`build/ci/scripts/determine-test-scope.ps1`	Detects Apple-related changes.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012psuk5dxG4cNcryRVL2VWS

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 1

Open (2)

The stageDependencies output variable reference format looks incorrect for Azure DevOps stage… · New
Include shared iOS resource tasks in tvOS path matching

github-actions · 2026-10-03T09:32:25Z

Claude finished @MartinZikmund's task in 2m 3s —— View job

Review complete ✅

Gather context (PR diff, AGENTS.md rules)
Review determine-test-scope.ps1 changes
Review .azure-devops-tests-skia-stages.yml changes
Post inline findings
Post summary

Verdict: Ship (1 confirmed non-blocking finding, 1 prior non-blocking finding still outstanding)

What I checked

The diff is unchanged since the last review pass (same two files, same SHAs). I re-verified the change end-to-end rather than re-stating prior output:

The new TvOSTestsRequired condition on runtime_tests_skia_tvos (.azure-devops-tests-skia-stages.yml:103) uses the exact stageDependencies.Setup.outputs['DetermineScope.SetScope.*'] idiom already established for TemplateTestsRequired (.azure-devops-stages.yml:78), including matching dependsOn: Setup and succeeded() (not succeededOrFailed()) — correct, since this stage's dependency shape mirrors the template-tests stage, not the screenshots stage.
Confirmed the job/task names (DetermineScope/SetScope) referenced in the new condition actually exist and match.
Confirmed the fallback paths (non-PR builds, git-fetch failure) enable every scope including the new one for free, since it's just another key in the same ordered hashtable — no special-casing needed or missed.
Verified the confirmed CI paths (.azure-devops-tests-tvos-skia*.yml, tvos-uitest-run.sh, skia-tvos-uitest-build.sh) all contain tvos with proper separator boundaries, so they match the new regex as intended.

New finding posted inline

Confirmed Copilot's review comment on determine-test-scope.ps1:40 with concrete evidence: the regex's separator-after-keyword requirement means PascalCase filenames like iOSResourceConverter.cs and iOSResourcesWriter.cs (under src/SourceGenerators/Uno.UI.Tasks/) aren't matched, even though Uno.UI.Tasks.targets:70 routes both iOS and tvOS target platforms through the resource pipeline these files implement. Non-blocking: both paths already trip TemplateTestsRequired via the existing uno\.ui\.tasks prefix, so there's no fully-silent gap — just not the tvOS-specific signal.

Cross-cutting, still open from the prior pass

The self-validation gap raised previously (the TvOSTestsRequired pattern doesn't match edits to .azure-devops-tests-skia-stages.yml itself, so future changes to the tvOS gating condition can't exercise the lane they gate) remains valid and unaddressed. Still non-blocking — this PR's description already acknowledges tvOS shows as skipped on its own run — but worth folding in before the pattern is relied on for self-validating future CI changes.

No C#/XAML/DependencyProperty/event-pattern surfaces are touched by this PR, so the rest of the AGENTS.md checklist doesn't apply.
· branch

unodevops · 2026-10-03T09:45:11Z

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24950/wasm-skia-net9/index.html

Match iOSResourceConverter-style names, and run tvOS when the stage condition or the scope script itself changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CKYPsdeNEWd1CARAojRx7H

github-actions · 2026-10-05T10:04:06Z

Claude finished @MartinZikmund's task in 2m 48s —— View job

Review complete ✅

Gather context (PR diff, AGENTS.md rules)
Review determine-test-scope.ps1 changes
Review .azure-devops-tests-skia-stages.yml changes
Post inline findings
Post summary

Verdict: Ship (1 new non-blocking observation; all prior findings resolved)

What's new since the last review pass

Commit d1e5ec94 ("Widen the tvOS scope to PascalCase and its gate") directly addresses both non-blocking findings from the previous rounds:

PascalCase gap (Copilot + confirmed by prior pass): the regex now adds a case-sensitive branch (?:^|[/._-])(?-i:iOS|tvOS|UIKit|Apple)(?-i:[A-Z]) that catches PascalCase identifiers like iOSResourceConverter.cs / iOSResourcesWriter.cs, where the keyword is followed by another identifier character rather than a separator. I traced this through manually: the (?-i:...) groups correctly override the pattern's leading (?i) to require exact casing plus a genuine uppercase word boundary, so it matches the previously-missed files without introducing false positives on adversarial cases I re-checked (scenarios.cs, ratios.cs, radios.cs, portfolios.cs, pineapple.cs, bios.cfg, ApplesauceHelper.cs — none match, as expected).
Self-validation gap (flagged by the prior automated pass): the pattern now explicitly matches ^build/ci/tests/\.azure-devops-tests-skia-stages\.yml$ and ^build/ci/scripts/determine-test-scope\.ps1$, so future edits to the gating condition or the scope script itself will trigger the tvOS lane they affect.

The stageDependencies.Setup.outputs['DetermineScope.SetScope.TvOSTestsRequired'], 'true' condition is unchanged from the first pass and I re-confirmed it's correct: it's byte-for-byte the same idiom already proven at build/ci/.azure-devops-stages.yml:78 and :138, and dependsOn: Setup on the tvOS stage matches the stageDependencies.Setup reference. Copilot's "high" severity concern about this format in its second pass is not a real issue — it matches the repo's existing, working convention.

New finding posted inline

One narrower, non-blocking residual gap on determine-test-scope.ps1:42: the explicit self-reference covers the two files this PR touches, but not build/ci/.azure-devops-stages.yml where the DetermineScope/SetScope job and task names are actually defined. A future rename there wouldn't be self-validated by the tvOS lane (it would trip TemplateTestsRequired instead, via the generic ^build/ prefix). Flagged for completeness, not blocking.

No source/test code, DependencyProperty, XAML, or event-pattern surfaces are touched by this PR, so the rest of the AGENTS.md checklist doesn't apply.
· branch

Copilot

Copilot review overview

🟢 Approval recommended

The implementation meets the linked issue; only the PR’s self-validation expectation needs correction.

Review effort: Balanced
Findings: 1

Open (1)

PR description incorrectly expects tvOS tests to be skipped · New

Resolved since last review (2)

The stageDependencies output variable reference format looks incorrect for Azure DevOps stage…
Include shared iOS resource tasks in tvOS path matching

 $patterns = @{
    TemplateTestsRequired = [regex]'(?i)(?:^build/|\.csproj$|\.props$|\.targets$|^src/uno\.sdk/|^src/sourcegenerators/uno\.ui\.tasks/|^src/.*devserver.*|^src/.*remotecontrol.*)'
    ScreenshotsRequired   = [regex]'(?i)^src/(uno\.ui|addins).*\.cs$'
+    TvOSTestsRequired     = [regex]'(?i)(?:^|[/._-])(?:tvos|ios|uikit|appleuikit|apple)(?:[/._-]|$)|(?:^|[/._-])(?-i:iOS|tvOS|UIKit|Apple)(?-i:[A-Z])|^build/ci/tests/\.azure-devops-tests-skia-stages\.yml$|^build/ci/scripts/determine-test-scope\.ps1$'


github-actions · 2026-10-05T10:06:58Z

 $patterns = @{
    TemplateTestsRequired = [regex]'(?i)(?:^build/|\.csproj$|\.props$|\.targets$|^src/uno\.sdk/|^src/sourcegenerators/uno\.ui\.tasks/|^src/.*devserver.*|^src/.*remotecontrol.*)'
    ScreenshotsRequired   = [regex]'(?i)^src/(uno\.ui|addins).*\.cs$'
+    TvOSTestsRequired     = [regex]'(?i)(?:^|[/._-])(?:tvos|ios|uikit|appleuikit|apple)(?:[/._-]|$)|(?:^|[/._-])(?-i:iOS|tvOS|UIKit|Apple)(?-i:[A-Z])|^build/ci/tests/\.azure-devops-tests-skia-stages\.yml$|^build/ci/scripts/determine-test-scope\.ps1$'


Nice fix — this resolves both gaps flagged in the prior review round (PascalCase iOSResourceConverter.cs/iOSResourcesWriter.cs now match via the case-sensitive (?-i:iOS|tvOS|UIKit|Apple)(?-i:[A-Z]) branch, and the self-reference gap is closed for the two files this PR actually touches).

One residual, non-blocking gap: the explicit self-reference only lists determine-test-scope.ps1 and .azure-devops-tests-skia-stages.yml. The condition on the tvOS stage also depends on the DetermineScope/SetScope job/task names being correct in build/ci/.azure-devops-stages.yml (where that job is defined — see build/ci/.azure-devops-stages.yml:29,43). A future rename of that job/task wouldn't be caught, because changes under build/ are covered by TemplateTestsRequired, not TvOSTestsRequired, so the tvOS lane itself would stay skipped on such a PR. Not worth blocking on — just flagging since this is the same class of self-validation issue already addressed for the other two files.

unodevops · 2026-10-05T10:27:40Z

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-24950/wasm-skia-net9/index.html

Copilot AI balanced review requested due to automatic review settings October 2, 2026 16:34

github-actions Bot added the area/build Categorizes an issue or PR as relevant to build infrastructure label Oct 2, 2026

github-actions Bot assigned MartinZikmund Oct 2, 2026

Copilot started reviewing on behalf of MartinZikmund October 2, 2026 16:35 View session

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Comment thread build/ci/scripts/determine-test-scope.ps1 Outdated

github-actions Bot reviewed Oct 2, 2026

View reviewed changes

Comment thread build/ci/scripts/determine-test-scope.ps1 Outdated

ci: Run tvOS on PRs only for Apple changes

a80e15a

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012psuk5dxG4cNcryRVL2VWS

Copilot AI balanced review requested due to automatic review settings October 3, 2026 08:52

MartinZikmund force-pushed the dev/mazi/ci-tvos-master-only branch from f28b860 to a80e15a Compare October 3, 2026 08:52

Copilot AI reviewed Oct 3, 2026

View reviewed changes

Comment thread build/ci/tests/.azure-devops-tests-skia-stages.yml

github-actions Bot reviewed Oct 3, 2026

View reviewed changes

Comment thread build/ci/scripts/determine-test-scope.ps1 Outdated

Copilot started reviewing on behalf of MartinZikmund October 3, 2026 09:36 View session

Copilot AI balanced review requested due to automatic review settings October 5, 2026 10:03

unoplatform deleted a comment from unodevops Oct 5, 2026

Copilot started reviewing on behalf of MartinZikmund October 5, 2026 10:04 View session

Copilot AI reviewed Oct 5, 2026

View reviewed changes

github-actions Bot reviewed Oct 5, 2026

View reviewed changes

Navigation Menu

ci: Run tvOS on PRs only for Apple changes - #24950

ci: Run tvOS on PRs only for Apple changes#24950
MartinZikmund wants to merge 2 commits into
masterfrom
dev/mazi/ci-tvos-master-only

MartinZikmund commented Oct 2, 2026

Uh oh!

github-actions Bot commented Oct 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Oct 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

unodevops commented Oct 3, 2026

Uh oh!

github-actions Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot Oct 5, 2026

Uh oh!

unodevops commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

MartinZikmund commented Oct 2, 2026

PR Type:

What changed? 🚀

PR Checklist ✅

Uh oh!

github-actions Bot commented Oct 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review complete ✅

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

github-actions Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review complete ✅

Uh oh!

Uh oh!

unodevops commented Oct 3, 2026

Uh oh!

github-actions Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review complete ✅

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

github-actions Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

unodevops commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 2, 2026 •

edited

Loading

github-actions Bot commented Oct 3, 2026 •

edited

Loading

github-actions Bot commented Oct 5, 2026 •

edited

Loading