Do less copying / zipping for Helix #40134
Conversation
dougbu
added
the
area-infrastructure
dougbu
requested review from
BrennanConroy,
halter73 and
Pilchie
as code owners
|
/fyi
|
| <PropertyGroup> | ||
| <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
| <RootNamespace>Microsoft.AspNetCore</RootNamespace> | ||
| <VerifyAncmBinary Condition="'$(TargetOsName)' == 'win' AND '$(TargetArchitecture)' != 'arm'">true</VerifyAncmBinary> | ||
| <TestDependsOnAspNetPackages>true</TestDependsOnAspNetPackages> |
There was a problem hiding this comment.
One thing to consider is to just stop running these special app tests on helix if these are now the only ones that require all this custom packaging, and just run these on the build agents once (say the windows test job). Might save us a bunch of grief in the long term maintaining this stuff if we no longer need it for any of the other normal tests
There was a problem hiding this comment.
One thing to consider is to just stop running these special app tests on helix if these are now the only ones that require all this custom packaging, and just run these on the build agents once (say the windows test job). Might save us a bunch of grief in the long term maintaining this stuff if we no longer need it for any of the other normal tests
There's now very little infrastructure for this and any move away from Helix is a regression. In addition, the project template tests need more stuff than these tests.
| @@ -79,9 +78,12 @@ | |||
| <HelixCorrelationPayload Include="$(HelixTestConfigurationFilePath)" AsArchive="false" /> | |||
| </ItemGroup> | |||
|
|
|||
| <Target Name="IncludeAspNetRuntime" BeforeTargets="Gather" | |||
| Condition="'$(DoNotRequireSharedFxHelix)' != 'true' OR | |||
| EXISTS('$(RepoRoot)artifacts\packages\$(Configuration)\Shipping\Microsoft.AspNetCore.App.Runtime.$(TargetRuntimeIdentifier).$(SharedFxVersion).nupkg')"> | |||
There was a problem hiding this comment.
@BrennanConroy I realize this line didn't work despite my insistence on including it in your $(DoNotRequireSharedFxHelix) work. $(SharedFxVersion) was never set which meant RunHelix.ps1 couldn't successfully run App.Unitest or the project template tests
dougbu
force-pushed
the
dougbu/app.unittest.fixes
branch
2 times, most recently
from
359dbcc to
3d76c90
Compare
- use .dotnet/ folders instead of App.Ref/App.Runtime packages for dotnet-cli layouts
- no need to unzip files into another layout
- fail fast or emit message when layouts don't exist
- set `$(TestDependsOnAspNetPackages)` to `false` by default
- no need to copy all packages into publish/ folders of most test projects
- upload very few times per queue (2 today, maybe 4 in the future), not for _every_ work item
- reduce the size of most work item payloads, saving bandwidth and time
- reduction should not cause problems, even around major version and TFM changes (see .dotnet/ folders)
- add `$(TestDependsOnAspNetAppPackages)` for the App.UnitTests case; just 2 packages needed there
- get App.UnitTests working in local builds
- grab RuntimeList.xml file from App.Runtime's obj/ folder
- find needed packages whether running locally or on Helix agents; should always exist
nits:
- remove test infrastructure we don't need anymore
- use one property for shared Fx and targeting pack versions; always the same
- remove `$env:ASPNET_RUNTIME_PATH`
- rename silly ...ListListsContains... tests
- remove extra `$(TestDependsOnAspNetXyz)` settings to their new default values
- add more comments
- use `$(ArtifactsShippingPackagesDir)` in Helix.targets - use `[MSBuild]::NormalizeDirectory(...)` and `$([System.IO.Path]::Combine(...)` in helix.proj
- explain a bit more about missing .dotnet/ layouts
dougbu
force-pushed
the
dougbu/app.unittest.fixes
branch
from
3d76c90 to
3032d6c
Compare
ghost
added this to the 
$(TestDependsOnAspNetPackages)tofalseby default$(TestDependsOnAspNetAppPackages)for the App.UnitTests case; just 2 packages needed therenits:
$env:ASPNET_RUNTIME_PATH$(TestDependsOnAspNetXyz)settings to their new default values