Skip to content

Use -noBuild in second build steps of test jobs #39998

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 8, 2022
Merged

Use -noBuild in second build steps of test jobs #39998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8eb2e11
Use `-noBuild` in second build steps of test jobs
dougbu Feb 4, 2022
898347f
Add comments per review feedback
dougbu Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .azure/pipelines/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ stages:
-noBuildRepoTasks
-noBuildNative
-noBuild
-noRestore
-sign
/p:DotNetSignType=$(_SignType)
$(_BuildArgs)
Expand Down Expand Up @@ -685,7 +684,8 @@ stages:
- script: ./eng/build.cmd -ci -nobl -all -pack -arch x64
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log $(_InternalRuntimeDownloadArgs)
displayName: Build shared fx
- script: ./eng/build.cmd -ci -nobl -noBuildRepoTasks -noRestore -test -all -noBuildNative
# -noBuildRepoTasks -noBuildNative -noBuild to avoid repeating work done in the previous step.
- script: ./eng/build.cmd -ci -nobl -all -noBuildRepoTasks -noBuildNative -noBuild -test
-projects eng\helix\helix.proj /p:IsHelixPRCheck=true /p:IsHelixJob=true
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log $(_InternalRuntimeDownloadArgs)
displayName: Run build.cmd helix target
Expand Down
3 changes: 2 additions & 1 deletion .azure/pipelines/helix-matrix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ jobs:
- script: ./eng/build.cmd -ci -nobl -all -pack -arch x64
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Build shared fx
- script: .\eng\build.cmd -ci -nobl -noBuildRepoTasks -noRestore -test -all -noBuildNative
# -noBuildRepoTasks -noBuildNative -noBuild to avoid repeating work done in the previous step.
- script: .\eng\build.cmd -ci -nobl -all -noBuildRepoTasks -noBuildNative -noBuild -test
-projects eng\helix\helix.proj /p:IsHelixJob=true
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Run build.cmd helix target
Expand Down
20 changes: 12 additions & 8 deletions .azure/pipelines/quarantined-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ jobs:
timeoutInMinutes: 120
steps:
# Build the shared framework
- script: ./eng/build.cmd -ci -nobl -all -pack -arch x64
- script: ./eng/build.cmd -ci -nobl -all -noBuildJava -pack -arch x64
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Build shared fx
- script: ./eng/build.cmd -ci -nobl -noBuildRepoTasks -noRestore -test -all -noBuildJava -noBuildNative
# -noBuildRepoTasks -noBuildNative -noBuild to avoid repeating work done in the previous step.
- script: ./eng/build.cmd -ci -nobl -all -noBuildRepoTasks -noBuildNative -noBuild -noBuildJava -test
-projects eng\helix\helix.proj /p:IsHelixPRCheck=true /p:RunQuarantinedTests=true /p:IsHelixJob=true
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Run build.cmd helix target
Expand All @@ -79,9 +80,10 @@ jobs:
isAzDOTestingJob: true
enablePublishTestResults: false
steps:
- powershell: "& ./eng/build.ps1 -CI -nobl -all -pack -NoBuildJava"
- script: ./eng/build.cmd -ci -nobl -all -noBuildJava -pack
displayName: Build
- script: ./eng/build.cmd -ci -nobl -test -NoRestore -NoBuild -NoBuilddeps "/p:RunQuarantinedTests=true /p:SkipHelixReadyTests=true"
- script: ./eng/build.cmd -ci -nobl -all -noBuildRepoTasks -noBuildNative -NoBuild -noBuildJava -test
/p:RunQuarantinedTests=true /p:SkipHelixReadyTests=true
displayName: Run Quarantined Tests
continueOnError: true
- task: PublishTestResults@2
Expand Down Expand Up @@ -112,9 +114,10 @@ jobs:
isAzDOTestingJob: true
enablePublishTestResults: false
steps:
- bash: ./eng/build.sh --all --pack --ci --nobl --no-build-java
- bash: ./eng/build.sh --ci --nobl --all --no-build-java --pack
displayName: Build
- bash: ./eng/build.sh --no-build --ci --nobl --test -p:RunQuarantinedTests=true -p:SkipHelixReadyTests=true
- bash: ./eng/build.sh --ci --nobl --all --no-build-repo-tasks --no-build --no-build-java --test
-p:RunQuarantinedTests=true -p:SkipHelixReadyTests=true
displayName: Run Quarantined Tests
continueOnError: true
- task: PublishTestResults@2
Expand Down Expand Up @@ -146,9 +149,10 @@ jobs:
enablePublishTestResults: false
useHostedUbuntu: false
steps:
- bash: ./eng/build.sh --all --pack --ci --nobl --no-build-java
- bash: ./eng/build.sh --ci --nobl --all --no-build-java --pack
displayName: Build
- bash: ./eng/build.sh --no-build --ci --nobl --test -p:RunQuarantinedTests=true -p:SkipHelixReadyTests=true
- bash: ./eng/build.sh --ci --nobl --all --no-build-repo-tasks --no-build --no-build-java --test
-p:RunQuarantinedTests=true -p:SkipHelixReadyTests=true
displayName: Run Quarantined Tests
continueOnError: true
- task: PublishTestResults@2
Expand Down
5 changes: 3 additions & 2 deletions .azure/pipelines/quarantined-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ jobs:
timeoutInMinutes: 480
steps:
# Build the shared framework
- script: ./eng/build.cmd -ci -nobl -all -pack -arch x64
- script: ./eng/build.cmd -ci -nobl -all -noBuildJava -pack -arch x64
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Build shared fx
- script: ./eng/build.cmd -ci -nobl -noBuildRepoTasks -noRestore -test -all -noBuildJava -noBuildNative
# -noBuildRepoTasks -noBuildNative -noBuild to avoid repeating work done in the previous step.
- script: ./eng/build.cmd -ci -nobl -all -noBuildRepoTasks -noBuildNative -noBuild -noBuildJava -test
-projects eng\helix\helix.proj /p:RunQuarantinedTests=true /p:IsHelixJob=true
/p:CrossgenOutput=false /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log
displayName: Run build.cmd helix target
Expand Down
3 changes: 2 additions & 1 deletion eng/scripts/RunHelix.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ $env:BUILD_REPOSITORY_NAME="aspnetcore"
$env:SYSTEM_TEAMPROJECT="aspnetcore"

Write-Host -ForegroundColor Yellow "If running tests that need the shared Fx, run './build -pack -all' before this."
Write-Host -ForegroundColor Yellow "And if packing for a different platform, add '/p:CrossgenOutput=false'."
Write-Host -ForegroundColor Yellow "If everything is up-to-date, add '/p:NoBuild=true' to this command."
Write-Host -ForegroundColor Yellow "Or, if only the test project is out-of-date, add '/p:BuildProjectReferences=false'."
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BrennanConroy these possibilities should speed up use of this script even more than setting $(p:DoNotRequireSharedFxHelix) 😸


$HelixQueues = $HelixQueues -replace ";", "%3B"
dotnet msbuild $Project /t:Helix /p:TargetArchitecture="$TargetArchitecture" `
Expand Down
1 change: 0 additions & 1 deletion eng/targets/FunctionalTestWithAssets.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
<RelativeFolder>%(Filename)</RelativeFolder>
<!-- Whether to _only_ publish the test asset into the test project's folders. -->
<SkipBuild>false</SkipBuild>
<AdditionalProperties Condition=" '%(SkipBuild)' == 'true' ">NoBuild=true</AdditionalProperties>
</TestAssetProjectReference>
</ItemDefinitionGroup>
</Project>
17 changes: 15 additions & 2 deletions eng/targets/FunctionalTestWithAssets.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,25 @@
<Target Name="PublishTestAssets"
BeforeTargets="Publish;RunTests;VSTest"
Condition=" '@(TestAssetProjectReference->Count())' != 0 ">
<!-- Build and Publish test assets into folders relative to test project. -->
<ItemGroup>
<_ProjectsToPublish Include="@(TestAssetProjectReference)" />

<!-- Always Publish test assets relative to test project's output. -->
<_ProjectsToPublish AdditionalProperties="%(_ProjectsToPublish.AdditionalProperties);
OutputPath=$(OutputPath)%(RelativeFolder);
PublishDir=$(PublishDir)%(RelativeFolder)" />

<!-- Do not build when publishing if SkipBuild requested. -->
<_ProjectsToPublish Condition=" '%(SkipBuild)' == 'true' "
AdditionalProperties="%(_ProjectsToPublish.AdditionalProperties);NoBuild=true" />

<!--
Otherwise, both Build and Publish test assets into folders relative to test project's output. Reset
NoBuild because these projects aren't referenced anywhere else.
-->
<_ProjectsToPublish Condition=" '%(SkipBuild)' != 'true' "
AdditionalProperties="%(_ProjectsToPublish.AdditionalProperties);
NoBuild=false;
OutputPath=$(OutputPath)%(RelativeFolder)" />
</ItemGroup>

<!--
Expand Down
9 changes: 8 additions & 1 deletion src/Mvc/samples/MvcSandbox/MvcSandbox.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<PreserveCompilationContext>true</PreserveCompilationContext>
Expand All @@ -14,4 +13,12 @@
<Reference Include="Microsoft.Extensions.Logging.Debug" />
</ItemGroup>

<!-- Work around https://github.com/dotnet/sdk/issues/23777. Reset ContentWithTargetPath items if not building. -->
<Target Name="RemoveCshtmlFiles" BeforeTargets="GetCopyToPublishDirectoryItems" Condition=" '$(NoBuild)' == true ">
<ItemGroup>
<ContentWithTargetPath>
<CopyToPublishDirectory Condition=" '%(Extension)' == '.cshtml' ">None</CopyToPublishDirectory>
</ContentWithTargetPath>
</ItemGroup>
</Target>
</Project>
3 changes: 2 additions & 1 deletion src/Mvc/test/Mvc.FunctionalTests/Microsoft.AspNetCore.Mvc.FunctionalTests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
<ProjectReference Include="..\..\shared\Mvc.Core.TestCommon\Microsoft.AspNetCore.Mvc.Core.TestCommon.csproj" />
<ProjectReference Include="..\WebSites\*\*.csproj"
Exclude="..\WebSites\ControllersFromServicesClassLibrary\ControllersFromServicesClassLibrary.csproj;
..\WebSites\RazorBuildWebSite.*\RazorBuildWebSite.*.csproj" />
..\WebSites\RazorBuildWebSite.*\RazorBuildWebSite.*.csproj;
..\WebSites\RazorPagesClassLibrary\RazorPagesClassLibrary.csproj" />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit I noticed while in the neighbourhood


<Reference Include="AngleSharp" />
<Reference Include="System.Net.Http" />
Expand Down
4 changes: 2 additions & 2 deletions src/Mvc/test/WebSites/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
-->
<StaticWebAssetsEnabled>false</StaticWebAssetsEnabled>
</PropertyGroup>
<!-- Skip the parent folder to prevent getting test package references. -->
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\..\, Directory.Build.props))\Directory.Build.props" />

<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" />
</Project>
14 changes: 14 additions & 0 deletions src/Mvc/test/WebSites/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.targets))\Directory.Build.targets" />

<!-- Work around https://github.com/dotnet/sdk/issues/23777. Reset ContentWithTargetPath items if not building. -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugly bug I noticed while working on this PR and this workaround likely doesn't cover all the real-world cases. /cc @marcpopMSFT @dsplaisted

<Target Name="RemoveCshtmlFiles"
BeforeTargets="GetCopyToPublishDirectoryItems"
Condition=" '$(RazorCompileOnPublish)' != false AND '$(NoBuild)' == true ">
<ItemGroup>
<ContentWithTargetPath>
<CopyToPublishDirectory Condition=" '%(Extension)' == '.cshtml' ">None</CopyToPublishDirectory>
</ContentWithTargetPath>
</ItemGroup>
</Target>
</Project>
1 change: 0 additions & 1 deletion src/Servers/IIS/IIS/test/Common.LongTests/StartupTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ public async Task StartsWithPortableAndBootstraperExe()
// We need the right dotnet on the path in IIS
deploymentParameters.EnvironmentVariables["PATH"] = Path.GetDirectoryName(DotNetCommands.GetDotNetExecutable(deploymentParameters.RuntimeArchitecture));

// ReferenceTestTasks is workaround for https://github.com/dotnet/sdk/issues/2482
var deploymentResult = await DeployAsync(deploymentParameters);

Assert.True(File.Exists(Path.Combine(deploymentResult.ContentRoot, "InProcessWebSite.exe")));
Expand Down
19 changes: 9 additions & 10 deletions src/Servers/IIS/IIS/test/FunctionalTest.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,24 @@
<Project>

<ItemGroup>
<Content Include="..\Common.FunctionalTests\Infrastructure\*.config" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>

<Target Name="BuildAssets" AfterTargets="Build" Condition="'$(ExcludeFromBuild)' != 'true'">
<MSBuild Projects="@(ProjectReference)" Targets="PublishTestsAssets" SkipNonexistentTargets="true" BuildInParallel="True">
<Target Name="CopyAssets" BeforeTargets="Publish" Condition=" '$(ExcludeFromBuild)' != true ">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing the two parts in separate targets worked when Publish always depended on Build

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any noticeable change here in behavior that we need to be aware of or is this just implementation details?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing I could detect because the published files from the test assets aren't used in local tests.

<MSBuild Projects="@(ProjectReference)"
Targets="PublishTestsAssets"
SkipNonexistentTargets="true"
BuildInParallel="True">
<Output TaskParameter="TargetOutputs" ItemName="PublishedTestAsset" />
</MSBuild>
</Target>

<Target Name="CopyAssets" AfterTargets="Publish" Condition="'@(PublishedTestAsset->Count())' != '0'">

<ItemGroup>
<ItemGroup Condition=" @(PublishedTestAsset->Count()) != 0 ">
<_FilesToCopy Include="%(PublishedTestAsset.Path)\**\*">
<DestinationDir>$(PublishDir)\%(PublishedTestAsset.Identity)\</DestinationDir>
</_FilesToCopy>
</ItemGroup>

<Copy SourceFiles="@(_FilesToCopy)" DestinationFiles="@(_FilesToCopy->'%(DestinationDir)%(RecursiveDir)%(FileName)%(Extension)')" />

<Copy SourceFiles="@(_FilesToCopy)"
Condition=" @(PublishedTestAsset->Count()) != 0 "
DestinationFiles="@(_FilesToCopy->'%(DestinationDir)%(RecursiveDir)%(FileName)%(Extension)')" />
</Target>

</Project>
2 changes: 1 addition & 1 deletion src/Servers/IIS/IIS/test/testassets/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<MSBuild Projects="$(MSBuildProjectFullPath)"
Targets="PublishTestsAsset"
RemoveProperties="Platform;PlatformTarget"
Properties="PublishDir=$(PublishDir);TestAssetProfile=%(TestAssetPublishProfile.Identity);ReferenceTestTasks=false;%(TestAssetPublishProfile.Properties)">
Properties="PublishDir=$(PublishDir);TestAssetProfile=%(TestAssetPublishProfile.Identity);%(TestAssetPublishProfile.Properties)">
<Output TaskParameter="TargetOutputs" ItemName="PublishedTestAsset" />
</MSBuild>
</Target>
Expand Down
15 changes: 14 additions & 1 deletion src/Servers/IIS/IIS/test/testassets/InProcessNewShimWebSite/InProcessNewShimWebSite.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@

<ItemGroup Condition="'$(OS)' == 'Windows_NT'">
<TestAssetPublishProfile Include="Portable" Properties="TargetFramework=$(DefaultNetCoreTargetFramework)" />
<TestAssetPublishProfile Condition="'$(TargetArchitecture)' != 'arm'" Include="Standalone-x64" Properties="RuntimeIdentifier=win-x64;" />

<!-- ReferenceTestTasks is workaround for https://github.com/dotnet/sdk/issues/2482 -->
<TestAssetPublishProfile Include="Standalone-x64"
Condition=" '$(TargetArchitecture)' != 'arm' "
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false" />
</ItemGroup>

<ItemGroup>
Expand Down Expand Up @@ -59,4 +63,13 @@
</PackageReference>
<Reference Include="xunit.assert" />
</ItemGroup>

<!-- Repeat Build target for win-x64 to allow later Publish w/ NoBuild=true. -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just confirming that this already covers the Win32/arm64 cases as well, or if I'll need to revisit this section for arm64 later

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add an arm64 profile to the @(TestAssetPublishProfile) item group, will need another target here too. Not sure how much the Condition will need to change w/o seeing what profile(s) you need to test. Of course, tests might have enough coverage w/o doing the self-contained thing for each possible Windows platform.

<Target Name="BuildX64"
BeforeTargets="Build"
Condition=" '$(RuntimeIdentifier)' != 'win-x64' AND '$(TargetArchitecture)' != 'arm' ">
<MSBuild Projects="$(MSBuildProjectFullPath)"
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false"
Targets="Build" />
</Target>
</Project>
15 changes: 13 additions & 2 deletions src/Servers/IIS/IIS/test/testassets/InProcessWebSite/InProcessWebSite.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<Import Project="..\..\..\..\build\testsite.props" />

<PropertyGroup>
Expand All @@ -9,7 +8,11 @@

<ItemGroup Condition="'$(OS)' == 'Windows_NT'">
<TestAssetPublishProfile Include="Portable" Properties="TargetFramework=$(DefaultNetCoreTargetFramework)" />
<TestAssetPublishProfile Condition="'$(TargetArchitecture)' != 'arm'" Include="Standalone-x64" Properties="RuntimeIdentifier=win-x64;" />

<!-- ReferenceTestTasks is workaround for https://github.com/dotnet/sdk/issues/2482 -->
<TestAssetPublishProfile Include="Standalone-x64"
Condition=" '$(TargetArchitecture)' != 'arm' "
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false" />
</ItemGroup>

<ItemGroup>
Expand All @@ -32,4 +35,12 @@
<Reference Include="xunit.assert" />
</ItemGroup>

<!-- Repeat Build target for win-x64 to allow later Publish w/ NoBuild=true. -->
<Target Name="BuildX64"
BeforeTargets="Build"
Condition=" '$(RuntimeIdentifier)' != 'win-x64' AND '$(TargetArchitecture)' != 'arm' ">
<MSBuild Projects="$(MSBuildProjectFullPath)"
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false"
Targets="Build" />
</Target>
</Project>