-
Notifications
You must be signed in to change notification settings - Fork 386
Lowering Build Sample Apps and Test Sample Apps from Aggregator to Foundation #5783
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
Outdated
Show resolved
Hide resolved
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
Outdated
Show resolved
Hide resolved
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
Outdated
Show resolved
Hide resolved
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Regarding "default setting is we don't run Build and Test Sample Apps in PR build, but run them in Nightly and Official", it sounds like a good start. If there is demand, we can try testing sample apps on 1 x64 OS image and 1 arm64 OS image in a PR pipeline run, to keep the workload small.
- Regarding "Q1: ... Will this target change affect the final test result?", my understanding is that, although we are exercising the same sample app as in the Agg pipeline, here we would be building the sample app differently, and therefore we are also testing a different launch scenario. That might still be a legitimate scenario to exercise, if we clearly document this difference. BTW, are the SelfContainedDeployment-* sample apps affected by this issue?
- Regarding "Q2: Shall we bring Windows SDK 10.0.22000 back to build/scripts/windows-sdk.ps1 or modify the sample app's target build platform?", my vote is to modify the sample app's target build platform, because to my understanding is that everyone should shift away from "Windows SDK 10.0.22000" overtime. In the meantime, I presume "installing Windows SDK 22000 ourselves in sample app build jobs in the pipeline" can be used as a short term workaround.
Thank you very much for this great work!
… remove FallbackSource, and add back comments
This issue have been resolved by Mano's recent Sample repo PR Thank you for your insights Alex! |
For this point, I've talked with Mano and realized that it is by design for us to only run in SelfContained mode if our application only use component package, as there is no framework package associated with the component package. So I added a comment to document the sample apps are run in SelfContained mode. Please help take a look whether this makes sense to you, thank you! @alexlamtest |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR lowers Sample App testing from the Aggregator repository to the Foundation repository to detect Foundation code errors earlier in the development cycle. The implementation adds comprehensive build and test infrastructure for sample applications while maintaining compatibility with existing Foundation pipeline patterns.
Key Changes:
- Build infrastructure: Adds sample app building capabilities with Foundation package references
- Test infrastructure: Implements sample app launch testing through TAEF test framework
- Pipeline integration: Integrates sample app stages into Foundation pipeline variants (PR, Nightly, Official)
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
test/WindowsAppSDK.Test.SampleTests/* | New test project with TAEF-based sample app launch tests |
test/WindowsAppSDK.Test.NetCore/* | Helper project for .NET Core runtime binaries |
test/ModifySampleAppsReferences.ps1 | Script to update sample app package references to Foundation packages |
build/AzurePipelinesTemplates/WindowsAppSDK-BuildSamplesCompat-Job.yml | Job template for building sample apps with Foundation packages |
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml | Stage template organizing sample app build and test workflows |
build/-Foundation-.yml | Pipeline configuration updates adding sample app parameters and stages |
TestAll.ps1 | Updated test runner with filtering logic for sample app tests |
Comments suppressed due to low confidence (1)
test/ModifySampleAppsReferences.ps1:1
- The PowerShell variable reference syntax is incorrect. It should be
$(variables['wasdkDependencies'])
to properly expand the variable value.
# This script is a Foundation version of https://github.com/microsoft/WindowsAppSDK-Samples/blob/main/UpdateVersions.ps1
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Proposal
This PR aims to lower the Sample App Tests (Build sample apps and test launch them) to Foundation to find the Foundation's code error earlier.
Note: This PR's change will bring much running time to pipeline. So our default setting is we don't run Build and Test Sample Apps in PR build, but run them in Nightly and Official
Changes in this PR
Build Sample Apps
build/AzurePipelinesTemplates/WindowsAppSDK-BuildSamplesCompat-Job.yml
- Mainly copied from Aggregator, the major changes are:IsOneBranch
param (line 4). Using this param to decide the pool (line 37) and publish build artifact (line 377)UseDotNet@2
task to not using global json (line 142)WindowsAppSDKVerifyTransitiveDependencies
for VSBuild command (line 233 and line 249) since there is no need for ML packageSDLNativeRules@3
except cpp-console-unpackaged (line 276) since other .sln will not be built at Foundationbuild/WindowsAppSDK-CommonVariables.yml
:test/ModifySampleAppsReferences.ps1
- Modified from Samples, the major changes are:Test Sample Apps
TestAll.ps1
:callingStage
param and filter the tests to runWindowsAppRuntime.sln
test/WindowsAppSDK.Test.NetCore/
folder and its contents - Mainly copied from Aggregatortest/WindowsAppSDK.Test.SampleTests/
folder and its contents - Mainly copied from AggregatorWindowsAppSDKSampleAppTests.cs
and 2) InWindowsAppSDK.Test.SampleTests.csproj
we set<ManagePackageVersionsCentrally>false</ManagePackageVersionsCentrally>
to befalse
as Foundation originally uses centralized version control. And we removed the package reference for WindowsAppSDKbuild/AzurePipelinesTemplates/WindowsAppSDK-RunTests-Steps.yml
- Mainly copied from Aggregator (This PR's line 97 - line 157 matches Aggregator's line 117 - line 200), the major changes are:net6
folder generated byWindowsAppSDK.Test.NetCore
at line 218, to make theTestAssembly.cs
property[TestProperty("CoreClrProfile", "net6")]
workbuild/AzurePipelinesTemplates/WindowsAppSDK-RunTestsInPipeline-Job.yml
:Other Files
build/AzurePipelinesTemplates/WindowsAppSDK-BuildAndTestSampleApps-Stage.yml
:BuildSampleApps
andTestSampleApps
sampleFeatureAreasList
is slightly different compared with Aggregator. The major difference is in Foundation we only keep the folder that all of its content could be successfully buliltbuild/AzurePipelinesTemplates/WindowsAppSDK-Publish-Stage.yml
PublishToMaestro
stage will be blocked byTestSampleApps
stage. By default the TestSampleApps will not affect the PublishToMaestro, if pipeline user want to have such dependency, he could manually tick theMaestroDependOnTestSamples
build/ProjectReunion-BuildFoundation.yml
build/WindowsAppSDK-Foundation-Nightly.yml
build/WindowsAppSDK-Foundation-Official.yml
build/WindowsAppSDK-Foundation-PR.yml
Pending discussion
WindowsAppSDK
toWindowsAppSDK.Foundation
. Therefore the dependency of packageWindowsAppSDK.Runtime
is removed. This causes MicrosoftWindowsAppSDKPackageDir still empty, then cause WindowsAppSDKSelfContained to betrue
, when this target being imported to Sample Apps build code, for example Notification/Push, this finally causes theWindowsAppSDK-Nuget-Native.targets
go to UndockedRegFreeWinRTCommon.targets path but not BootstrapCommon.targets as Aggregator does. Will this target change affect the final test result?build/scripts/windows-sdk.ps1
. However, some of our sample apps require Windows SDK 10.0.22000 as target build platform. Shall we bring Windows SDK 10.0.22000 back tobuild/scripts/windows-sdk.ps1
or modify the sample app's target build platform?Example Run for this PR (since PR build will not run Build and Test Sample Apps by default, we run it manually)
Succeed run without commit clean up:
https://microsoft.visualstudio.com/ProjectReunion/_build/results?buildId=129369163&view=results
After commit clean up and rebase main (not totally succeed because of the second point in Pending Discussion section)
https://microsoft.visualstudio.com/ProjectReunion/_build/results?buildId=129381070