Repository navigation
Correct Test-DotnetNewTemplates.ps1 comments that misdescribe app-template coverage - #6805
Alexandre Zollinger Chohfi (azchohfi) wants to merge 2 commits into
Conversation
…plate coverage Two comments in the template smoke test described behaviour the script does not have, both in the same direction: they implied the packaged path and `dotnet run` are covered when neither is. .DESCRIPTION claimed app templates "are optionally launched via dotnet run using the recommended WinAppSDK self-contained properties." Neither half is true. Launch-AppExecutables (line 265) resolves the built .exe under bin/ and Start-Process's it directly at line 277; `dotnet run` is never invoked. Builds pass -p:WindowsPackageType=None, which is the opposite of the self-contained packaged shape. The comment above the Reactor entries called those templates "unpackaged" and said "their .csproj already sets it" (WindowsPackageType). Both are wrong. The Reactor templates are packaged single-project MSIX apps: templates.props gives each one RenameManifestFrom=Package-managed.appxmanifest, and their ProjectTemplate.csproj sets EnableMsixTooling=true and no WindowsPackageType at all - the flag comes from the harness command line at line 372. That comment was never accurate rather than having drifted: 2fd7355 (#6620) introduced it in the same commit that added the templates, and at that commit all four ProjectTemplate.csproj files already declared EnableMsixTooling and no WindowsPackageType. It contradicted the templates it shipped with. This matters beyond tidiness. All nine Kind='App' templates ship a manifest and are built with packaging disabled, so Package.appxmanifest processing, MSIX tooling and identity activation are unexercised. Anyone auditing that coverage reads .DESCRIPTION, sees "launched via dotnet run", and stops. The comments were hiding the gap from the repo that owns it. No rationale for the flag is asserted here: none is recorded. It arrived with the harness in 9b5d40a (#6407), which added the file, so it was present from the first version rather than introduced later as a deliberate constraint. Comment-only; no behaviour change. Script parses with 0 errors. Line numbers above are as of this commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Separately, the comment-based help block opened with `<#+` instead of `<#`. That is a valid block comment to the parser, so the script always ran fine, but Get-Help could not recognise it as comment-based help: `Get-Help .\Test-DotnetNewTemplates.ps1` returned the auto-generated syntax line and no DESCRIPTION or PARAMETER text at all. Verified by copying the file to a scratch directory and changing only that token: as-is Get-Help reports no description, corrected it renders the full help including every .PARAMETER. That matters here because it is the reason the inaccurate .DESCRIPTION went unnoticed for so long - the corrected text would still have been invisible to anyone running Get-Help, which is how a PowerShell script is normally inspected.
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Superseded by a fresh PR with identical content on branch The change itself is unchanged (same blob, For the record, CI on this PR did eventually run in full (57 checks), and the stage that actually executes the edited script — "Smoke-test dotnet new templates / Scaffold and build dotnet new templates" — passed. The only failure was "Validate VSIX templates build / Restore + build WindowsAppSDK.Extension.sln", which fails identically on unrelated PRs #6843, #6835 and #6824, so it is a pre-existing repo-wide failure rather than anything introduced here. |
|
Superseded by #6846. |
Three comment-only corrections to the
dotnet newtemplate smoke test. No behaviour change to what the script builds or runs.1.
.DESCRIPTIONdescribed coverage the script doesn't haveIt claimed app templates "are optionally launched via dotnet run using the recommended WinAppSDK self-contained properties." Neither half is true:
Launch-AppExecutablesresolves the built.exeunderbin/andStart-Processes it directly.dotnet runis never invoked.-p:WindowsPackageType=None, which is the opposite of the self-contained packaged shape.2. The Reactor comment was false from the commit that wrote it
It called those templates "unpackaged" and said "their .csproj already sets it" (
WindowsPackageType). Both wrong — and not drift:2fd73557(#6620) added the comment and the templates in the same commit, and at that commit all fourProjectTemplate.csprojfiles already declaredEnableMsixTooling=truewith noWindowsPackageTypeelement. It contradicted the templates it shipped with.templates.propsalso gives each oneRenameManifestFrom=Package-managed.appxmanifest.3.
<#+suppressed comment-based help entirelyThe help block opened with
<#+instead of<#. That's a valid block comment to the parser, so the script always ran fine — butGet-Helpdidn't recognise it as comment-based help and returned the auto-generated syntax line with noDESCRIPTIONorPARAMETERtext.Verified by copying the file to a scratch directory and changing only that token:
Get-Helpoutput<#+(before)<#(after).PARAMETERThis is why the inaccurate
.DESCRIPTIONwent unnoticed: correcting the text alone would still have left it invisible to anyone inspecting the script the normal way. Fixing the opener also surfaces the existing-SkipAppLaunch,-Platforms,-RunTimeoutSecondsetc. help for the first time.Why this matters beyond tidiness
All nine
Kind='App'templates ship a manifest and are built with packaging disabled, soPackage.appxmanifestprocessing, MSIX tooling and identity activation are unexercised. Anyone auditing that coverage reads.DESCRIPTION, sees "launched via dotnet run", and stops. The comments were hiding the gap from the repo that owns it.No rationale for
-p:WindowsPackageType=Noneis asserted here, in either direction — none is recorded. It arrived with the harness in9b5d40a61(#6407), the commit that added the file, so it was present in the first version rather than introduced later as a deliberate constraint. Rather than invent a plausible justification, this PR just makes the description match observable behaviour.Validation
[Parser]::ParseFile).Get-Helprenders the corrected description and all parameter help.Note on CI coverage for this PR
This change is under
dev/Templates/**, which matches the+dev/Templates/**PR trigger filter added in #6782, soWindowsAppSDK-Templates-PR— the pipeline that actually runs this script viaWindowsAppSDK-DotnetNewTemplates-Test-Stage.yml— would be expected to run.At time of writing it has not appeared on this PR. Roughly 6 minutes after push, only
WinAppSDK-TouchDownBuildandlicense/claare reporting. For comparison, #6786 (also strictly underdev/Templates/**) ran 55 checks including "Smoke-test dotnet new templates / Scaffold and build dotnet new templates".This may simply be queue latency, but flagging it rather than asserting a run that hasn't happened. Worth confirming the templates smoke test actually executes before merging, since this PR edits the script that pipeline runs. (
WinAppSDK-TouchDownBuildfails identically on unrelated PR #6804, so that failure is pre-existing and not from this change.)