You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#1277 replaced the deleted CreateTemplateTests with a packaged scaffold → build → launch smoke in the Bootstrap job. It asserts that a freshly scaffolded app from the external template pack, built against the source-built 0.0.0-local packages, registers as a loose-layout MSIX, activates with identity, and renders its Reactor UI.
That is narrower than packaged testing in general: ci.yml's Packaged Selftests already run the in-repo host under MSIX identity on every non-markdown change. What only this job covers is the scaffolded template path: the template's own manifest, package graph and first render.
But the Bootstrap workflow is path-filtered, and framework sources were not in the list, so the smoke only ran when bootstrap's own inputs changed. The test it replaced lived in Reactor.IntegrationTests, gated on non-md, so it ran on any framework change. Coverage didn't disappear, but its reach narrowed: a framework regression that broke the scaffolded app's packaged startup, identity activation or first render would not have been caught.
What changed
.github/workflows/bootstrap.yml triggers, identical for pull_request and push:
src/**, default-on, minus the four projects that cannot affect a scaffolded app: Reactor.Compile.Analyzer, Reactor.Interop.WinForms, vs-reactor, vscode-reactor.
!**/*.md, so a doc-only edit under src/ doesn't spin a 12-minute job. Negations are order-sensitive and kept last; a mixed .cs + .md change still runs.
Why a denylist, not an allowlist
The first revision enumerated inputs (src/Reactor/**, src/Reactor.Devtools/**), and review found it incomplete twice. The scaffold's inputs are wider than they look:
the template references Microsoft.UI.ReactorandMicrosoft.UI.Reactor.Devtools, both at $ReactorVersion$;
Reactor.csproj packs six more projects into the package consumers compile against: Reactor.Analyzers, Reactor.Analyzers.Internal, the Localization / Wrappers / SourceMap generators, and Reactor.Wrappers.Abstractions.
That is 10 of the 14 projects under src/. An allowlist of them drifts silently whenever one is added, and a miss surfaces as an uncaught packaged regression, never as a red build. Default-on trades that for an occasional unnecessary run, which is the right way round for a job whose purpose is catching what nothing else does.
Why not move it into ci.yml
Considered and measured. The smoke's prerequisites (winapp CLI, the template pack, mur, and local-nupkgs/ with a source-built 0.0.0-local framework) are all supplied by bootstrap.ps1. Recreating them beside Integration Tests meant ~6 duplicated setup steps and ~10 min added to a 5m53s job, and because non-md is broader than these paths it would also fire on tests/, samples/ and tools/ changes. Same coverage, more duplication, less targeted.
Test plan
bootstrap.yml parses; pull_request and push path lists identical
Filter semantics modelled over 19 cases, 0 mismatches:
all ten package inputs trigger: Reactor, Reactor.Analyzers, Reactor.Analyzers.Internal, the three generators, Reactor.Wrappers.Abstractions, Reactor.Advanced, Reactor.Devtools, Reactor.Cli;
the four excluded projects, markdown under src/, docs/, tests/ and samples/ do not;
a mixed .cs + .md change still triggers.
The Bootstrap job ran green on every push here, still asserting the full chain:
Found via a cross-repo review from the WindowsAppSDK side. Its core claim, that nothing now covered the packaged path, turned out not to hold: this job does. Checking it surfaced the narrower trigger gap fixed here.
The upstream observation is accurate and worth recording: dev/Templates/Dotnet/Test-DotnetNewTemplates.ps1 builds every Kind='App' template with -p:WindowsPackageType=None (line 370) and Start-Processes the raw exe rather than using dotnet run, so the upstream harness validates the templates with packaging disabled.
PR #1277 replaced the deleted CreateTemplateTests with a packaged scaffold →
build → launch smoke in the Bootstrap job, which asserts what no other suite
does: that the loose-layout MSIX registers, the app activates with identity,
and the Reactor UI renders. Every other suite runs unpackaged.
But that job is path-filtered, and `src/Reactor/**` was not in the list — so the
smoke only ran when bootstrap's own inputs changed. The test it replaced lived
in Reactor.IntegrationTests, gated on `non-md`, so it ran on any framework
change. Coverage did not disappear, but its reach narrowed: a framework
regression that broke packaged startup, identity activation or first render
would not have been caught.
Adds `src/Reactor/**` and `src/Reactor.Devtools/**`. Devtools is not incidental
— the scaffolded template references both packages at `$ReactorVersion$`
(verified in the shipped template's ProjectTemplate.csproj), and CI builds the
app against the source-built 0.0.0-local packages, so a Devtools break fails the
smoke too.
Also excludes markdown, so a doc-only edit to one of the three .md files now
inside those paths doesn't spin a 12-minute job. Negations are order-sensitive
and kept last; a PR touching both a .cs and a .md still runs.
Verified by modelling GitHub's filter semantics over eight cases: framework,
devtools and bootstrap inputs trigger; doc-only under those paths does not;
mixed .cs + .md still triggers; unrelated docs, tests and samples do not.
Found via a cross-repo review from the WindowsAppSDK side, which noted that the
upstream template harness builds every App template with
`-p:WindowsPackageType=None` (dev/Templates/Dotnet/Test-DotnetNewTemplates.ps1
line 370) and launches the raw exe rather than `dotnet run` — so packaging is
validated in neither repo by that path. Reactor does own it, via this job; it
just wasn't watching the input most likely to break it.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b893b2b8-78f8-46c6-9227-515e1dc3a563
Artifact sizes for 3fbcd62 vs the base branch (6d783c5).
Packages (compressed .nupkg)
Artifact
base
PR
Δ
Microsoft.UI.Reactor.nupkg
1.72 MB
1.72 MB
-44 B (0.00%)
≈
Microsoft.UI.Reactor.Advanced.nupkg
487.6 KB
487.6 KB
-8 B (0.00%)
≈
Microsoft.UI.Reactor.Devtools.nupkg
284.1 KB
284.1 KB
-13 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor
Artifact
base
PR
Δ
Reactor.Analyzers.dll
373.0 KB
373.0 KB
+0 B (0.00%)
≈
Reactor.dll
2.44 MB
2.44 MB
+0 B (0.00%)
≈
Reactor.Localization.Generator.dll
16.0 KB
16.0 KB
+0 B (0.00%)
≈
Reactor.Wrappers.Abstractions.dll
10.5 KB
10.5 KB
+0 B (0.00%)
≈
Reactor.Wrappers.Generator.dll
99.5 KB
99.5 KB
+0 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor.Advanced
Artifact
base
PR
Δ
Reactor.Advanced.dll
1021.5 KB
1021.5 KB
+0 B (0.00%)
≈
Assemblies in Microsoft.UI.Reactor.Devtools
Artifact
base
PR
Δ
Microsoft.UI.Reactor.Devtools.dll
784.5 KB
784.5 KB
+0 B (0.00%)
≈
No size change beyond the noise floor. ✅
✅ smaller / ⚠️ larger / ≈ within noise. Sizes come from a Release dotnet pack on the CI runner: packages are the compressed .nupkg download size, assemblies the uncompressed DLL inside it. workflow run.
Coverage for 3fbcd62 vs the base branch (6d783c5) — unit + selftest merged.
Metric
base
PR
Δ
Line
85.79%
85.79%
0.00 pp
≈
Branch
77.58% (962/1240)
77.66% (963/1240)
+0.08 pp
≈
No coverage change beyond the noise floor. ✅
✅ higher / ⚠️ lower / ≈ within noise. Δ is in percentage points; coverage is unit + selftest merged (Debug x64) on the CI runner. Cobertura reports attached to the workflow run as artifacts.
Copilot review on #1289 — both findings correct.
Reactor.Devtools project-references Reactor.Advanced, and the scaffolded
template references Devtools, so an Advanced change flows into the package
graph this smoke builds and runs. Added to both trigger lists; verified the
reference in src/Reactor.Devtools/Reactor.Devtools.csproj rather than assuming
it from the pack-local output.
The comment also claimed "every other suite runs unpackaged", which is wrong:
ci.yml's Packaged Selftests run the in-repo host under MSIX identity on every
non-md change. Reworded to state the actual, narrower gap — the *scaffolded
template* path: a fresh app from the external template pack, built against
local packages, registering and rendering. That is what no other job covers.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b893b2b8-78f8-46c6-9227-515e1dc3a563
Include all Reactor package inputs in smoke-test filters
.github/workflows/bootstrap.yml:30
These filters still omit several projects that mur pack-local rebuilds into the exact Microsoft.UI.Reactor package consumed by the scaffold: Reactor.csproj references and packs Reactor.Analyzers, Reactor.Localization.Generator, Reactor.Wrappers.Generator, Reactor.SourceMap.Generator, and bundles Reactor.Wrappers.Abstractions. A change there can alter consumer compilation or package behavior without starting this smoke. Add the same package-input paths to both event lists (or use an equivalent explicit set).
Copilot review round 2 on #1289. The finding is right, and understates itself:
Reactor.csproj project-references six projects whose output is packed into the
Microsoft.UI.Reactor package consumers compile against — Analyzers,
Analyzers.Internal, and the Localization / Wrappers / SourceMap generators,
plus Wrappers.Abstractions. The review listed five; Analyzers.Internal is
referenced too.
The deeper problem is the shape, not the contents. Counting Reactor itself,
Devtools (referenced by the template), Advanced (project-referenced by
Devtools) and the CLI, that is 10 of the 14 projects under src/. My allowlist
had already drifted twice inside this one PR — it shipped without Advanced,
then without those six — and each miss is invisible: it surfaces as a packaged
regression nobody caught, never as a red build.
So the filter is now `src/**` minus the four that cannot affect a scaffolded
app: Reactor.Compile.Analyzer, Reactor.Interop.WinForms, vs-reactor,
vscode-reactor. A generator added to Reactor.csproj tomorrow is covered by
default; the failure mode becomes an occasional unnecessary 12-minute run
rather than a silent hole, which is the right way round for this job.
Verified over 19 cases, 0 mismatches: all ten package inputs trigger; the four
excluded projects, markdown, docs, tests and samples do not; a mixed .cs + .md
change still runs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b893b2b8-78f8-46c6-9227-515e1dc3a563
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#1277 replaced the deleted
CreateTemplateTestswith a packaged scaffold → build → launch smoke in the Bootstrap job. It asserts that a freshly scaffolded app from the external template pack, built against the source-built0.0.0-localpackages, registers as a loose-layout MSIX, activates with identity, and renders its Reactor UI.That is narrower than packaged testing in general:
ci.yml's Packaged Selftests already run the in-repo host under MSIX identity on every non-markdown change. What only this job covers is the scaffolded template path: the template's own manifest, package graph and first render.But the Bootstrap workflow is path-filtered, and framework sources were not in the list, so the smoke only ran when bootstrap's own inputs changed. The test it replaced lived in
Reactor.IntegrationTests, gated onnon-md, so it ran on any framework change. Coverage didn't disappear, but its reach narrowed: a framework regression that broke the scaffolded app's packaged startup, identity activation or first render would not have been caught.What changed
.github/workflows/bootstrap.ymltriggers, identical forpull_requestandpush:src/**, default-on, minus the four projects that cannot affect a scaffolded app:Reactor.Compile.Analyzer,Reactor.Interop.WinForms,vs-reactor,vscode-reactor.!**/*.md, so a doc-only edit undersrc/doesn't spin a 12-minute job. Negations are order-sensitive and kept last; a mixed.cs+.mdchange still runs.Why a denylist, not an allowlist
The first revision enumerated inputs (
src/Reactor/**,src/Reactor.Devtools/**), and review found it incomplete twice. The scaffold's inputs are wider than they look:Microsoft.UI.ReactorandMicrosoft.UI.Reactor.Devtools, both at$ReactorVersion$;Reactor.Devtoolsproject-referencesReactor.Advanced;Reactor.csprojpacks six more projects into the package consumers compile against:Reactor.Analyzers,Reactor.Analyzers.Internal, the Localization / Wrappers / SourceMap generators, andReactor.Wrappers.Abstractions.That is 10 of the 14 projects under
src/. An allowlist of them drifts silently whenever one is added, and a miss surfaces as an uncaught packaged regression, never as a red build. Default-on trades that for an occasional unnecessary run, which is the right way round for a job whose purpose is catching what nothing else does.Why not move it into
ci.ymlConsidered and measured. The smoke's prerequisites (winapp CLI, the template pack,
mur, andlocal-nupkgs/with a source-built0.0.0-localframework) are all supplied bybootstrap.ps1. Recreating them beside Integration Tests meant ~6 duplicated setup steps and ~10 min added to a 5m53s job, and becausenon-mdis broader than these paths it would also fire ontests/,samples/andtools/changes. Same coverage, more duplication, less targeted.Test plan
bootstrap.ymlparses;pull_requestandpushpath lists identicalReactor,Reactor.Analyzers,Reactor.Analyzers.Internal, the three generators,Reactor.Wrappers.Abstractions,Reactor.Advanced,Reactor.Devtools,Reactor.Cli;src/,docs/,tests/andsamples/do not;.cs+.mdchange still triggers.Context
Found via a cross-repo review from the WindowsAppSDK side. Its core claim, that nothing now covered the packaged path, turned out not to hold: this job does. Checking it surfaced the narrower trigger gap fixed here.
The upstream observation is accurate and worth recording:
dev/Templates/Dotnet/Test-DotnetNewTemplates.ps1builds everyKind='App'template with-p:WindowsPackageType=None(line 370) andStart-Processes the raw exe rather than usingdotnet run, so the upstream harness validates the templates with packaging disabled.