-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Add support for running helix tests against built shared fx #16828
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
Changes from all commits
1dc754a
d5f751d
df12fba
1372178
03952a5
71a97ea
a6f5355
8cbcec0
43e4461
d82c597
40ec31a
8af51de
c291b67
2786edc
56a4c2e
7bcadb2
d0e84f1
63726b1
1f05baf
c8418d9
c6af10c
e9f01e2
e9e8b09
0ecda9d
2a07f0b
782953a
90bae46
626e81a
de0fe18
15e858d
11e4ea8
3dee8b0
e4cac96
e9d17d8
c38f898
b2613db
99df82d
927d2d0
d60da6d
d5f473c
b82c025
62edbb3
74f9286
a95865a
495ee43
f7a8dd5
1e7d5d2
01e256f
e3e2ce8
fb60dfd
4a9cafb
cdc0cec
5f6bb8a
82f6b71
6465791
2b9cfce
419ec20
dd5d764
2be9211
815d336
87e8f10
be1d67b
65e8904
029148f
1cbe21a
0aad54b
f709295
fa2337c
3b73a98
3587793
d28d0eb
1147ec7
ffc6c3d
425d2d2
9e35b55
3417691
21eb87a
e4d039d
b154815
b97095b
38cbffe
c098d38
55b95d7
6ea8340
b58fb2d
7547e7a
3d07b4c
c4f2bcc
1704910
e38a0a5
d91f61d
c902007
c8b05dc
04c7bf0
7be4e77
d4a767b
785c64b
5e40ec7
c215cb3
d42e3e4
e33eb8c
3644935
7d05178
581dbe9
34b8d05
c4128d4
4b93786
c28b82f
7f23917
ccc5325
1dd3ed4
90f9629
f3cc4ba
99921d7
c2df878
5fe430e
647fa7d
6bed900
8133a27
dcdf5b0
0523a29
acda40b
48932a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,8 +125,7 @@ stages: | |
displayName: Build x64 | ||
|
||
# Build the x86 shared framework | ||
# TODO: make it possible to build for one Windows architecture at a time | ||
# This is going to actually build x86 native assets. See https://github.com/dotnet/aspnetcore/issues/7196 | ||
# This is going to actually build x86 native assets. | ||
- script: ./build.cmd | ||
-ci | ||
-arch x86 | ||
|
@@ -582,9 +581,16 @@ stages: | |
agentOs: Windows | ||
timeoutInMinutes: 180 | ||
steps: | ||
- script: .\restore.cmd -ci | ||
displayName: Restore | ||
- script: .\build.cmd -ci -NoRestore -test -projects eng\helix\helix.proj /p:IsRequiredCheck=true /p:IsHelixJob=true /p:BuildAllProjects=true /p:BuildNative=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log -bl | ||
# Build the shared framework | ||
- script: ./build.cmd -ci -all -pack -arch x64 /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log /bl:artifacts/log/helix.build.x64.binlog | ||
displayName: Build shared fx | ||
# Build the x86 shared framework | ||
# This is going to actually build x86 native assets. | ||
- script: ./build.cmd -ci -arch x86 -pack -all -buildNative -noBuildJava /p:OnlyPackPlatformSpecificPackages=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log /bl:artifacts/log/helix.build.x86.binlog $(_BuildArgs) | ||
displayName: Build x86 shared framework pieces | ||
- script: ./src/ProjectTemplates/build.cmd -ci -pack -NoRestore -NoBuilddeps "/p:RunTemplateTests=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log /bl:artifacts/log/helix.template.pack.binlog" | ||
displayName: Pack Templates | ||
- script: .\build.cmd -ci -noBuildRepoTasks -noRestore -noBuild -test -projects eng\helix\helix.proj /p:IsRequiredCheck=true /p:IsHelixJob=true /p:BuildAllProjects=true /p:BuildNative=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log -bl | ||
displayName: Run build.cmd helix target | ||
env: | ||
HelixApiAccessToken: $(HelixApiAccessToken) # Needed for internal queues | ||
|
@@ -603,9 +609,23 @@ stages: | |
agentOs: Windows | ||
timeoutInMinutes: 180 | ||
steps: | ||
- script: .\restore.cmd -ci | ||
displayName: Restore | ||
- script: .\build.cmd -ci -NoRestore -test -projects eng\helix\helix.proj /p:IsHelixJob=true /p:IsHelixDaily=true /p:BuildAllProjects=true /p:BuildNative=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log -bl | ||
- script: ./build.cmd -ci -all -pack -arch x64 /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log /bl:artifacts/log/helix.daily.build.x64.binlog | ||
displayName: Build shared fx | ||
# Build the x86 shared framework | ||
# This is going to actually build x86 native assets. | ||
- script: ./build.cmd | ||
-ci | ||
-arch x86 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be 'x64' for 64-bit testing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only saw an x86 shared framework in the ci.yml, so I assume it doesn't matter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Windows x64 / x86 build step's comments are a bit confusing. The first I'm surprised the tests succeeded with the x86 shared runtime. Does anything actually test using it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is on by default so all the shared framework bits are getting copied onto the helix machines for every test, so it doesn't seem to be causing any issues. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point is the created x86 shared framework here may not be used at all because it's not the bitness the tests need. Not causing issues isn't the same thing as enabling the testing we need. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure what's the fix just specify arch x64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest adding a test that won't run unless it executes against the expected shared framework. Maybe something that looks at the location of a loaded SharedFX assembly. That'll help show what's working (or not) here. Put another way, I'd rather not see this new infrastructure need further changes as soon as we try to use it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is prerequisite work for template testing, I don't think we need to add a new test for that, the only reason we need this is because its a requirement for templates which will fail if they aren't running against latest shared framework bits. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rewording what I said: This doesn't look like it should work. Need to show that it is working w/ Win x64 test. But, sure, one way to do that would be to enable template tests in the same PR. |
||
-pack | ||
-all | ||
-buildNative | ||
-noBuildJava | ||
/p:OnlyPackPlatformSpecificPackages=true | ||
/p:ASPNETCORE_TEST_LOG_DIR=artifacts/log | ||
/bl:artifacts/log/helix.daily.build.x86.binlog | ||
HaoK marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$(_BuildArgs) | ||
displayName: Build x86 shared framework pieces | ||
- script: .\build.cmd -ci -noBuildRepoTasks -noRestore -noBuild -test -projects eng\helix\helix.proj /p:IsHelixJob=true /p:IsHelixDaily=true /p:BuildAllProjects=true /p:BuildNative=true /p:ASPNETCORE_TEST_LOG_DIR=artifacts/log -bl | ||
displayName: Run build.cmd helix target | ||
env: | ||
HelixApiAccessToken: $(HelixApiAccessToken) # Needed for internal queues | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
<# | ||
.SYNOPSIS | ||
Installs an AspNetCore shared framework on a machine | ||
.DESCRIPTION | ||
This script installs an AspNetCore shared framework on a machine | ||
.PARAMETER AppRuntimePath | ||
The path to the app runtime package to install. | ||
.PARAMETER InstallDir | ||
The directory to install the shared framework to. | ||
.PARAMETER Framework | ||
The framework directory to copy the shared framework from. | ||
#> | ||
param( | ||
[Parameter(Mandatory = $true)] | ||
$AppRuntimePath, | ||
|
||
[Parameter(Mandatory = $true)] | ||
$InstallDir, | ||
|
||
[Parameter(Mandatory = $true)] | ||
$Framework) | ||
|
||
$ErrorActionPreference = 'Stop' | ||
$ProgressPreference = 'SilentlyContinue' # Workaround PowerShell/PowerShell#2138 | ||
|
||
Set-StrictMode -Version 1 | ||
|
||
Write-Host "Extracting to $InstallDir" | ||
|
||
$zipPackage = [io.path]::ChangeExtension($AppRuntimePath, ".zip") | ||
Write-Host "Renaming to $zipPackage" | ||
Rename-Item -Path $AppRuntimePath -NewName $zipPackage | ||
if (Get-Command -Name 'Microsoft.PowerShell.Archive\Expand-Archive' -ErrorAction Ignore) { | ||
# Use built-in commands where possible as they are cross-plat compatible | ||
Microsoft.PowerShell.Archive\Expand-Archive -Path $zipPackage -DestinationPath ".\tmpRuntime" -Force | ||
} | ||
else { | ||
Remove-Item ".\tmpRuntime" -Recurse -ErrorAction Ignore | ||
# Fallback to old approach for old installations of PowerShell | ||
Add-Type -AssemblyName System.IO.Compression.FileSystem | ||
[System.IO.Compression.ZipFile]::ExtractToDirectory($zipPackage, ".\tmpRuntime") | ||
} | ||
|
||
Get-ChildItem -Path ".\tmpRuntime" -Recurse | ||
|
||
Write-Host "Copying managed files to $InstallDir" | ||
Copy-Item -Path ".\tmpRuntime\runtimes\win-x64\lib\$Framework\*" $InstallDir | ||
Write-Host "Todo: Copying native files to $InstallDir" | ||
# Copy-Item -Path ".\tmpRuntime\runtimes\win-x64\native\*" $InstallDir |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#!/usr/bin/env bash | ||
|
||
# Cause the script to fail if any subcommand fails | ||
set -e | ||
|
||
appRuntimePath=$1 | ||
output_dir=$2 | ||
framework=$3 | ||
tmpDir=./tmpRuntime | ||
|
||
echo "Installing shared framework from $appRuntimePath" | ||
cp $appRuntimePath sharedFx.zip | ||
|
||
mkdir -p $tmpDir | ||
unzip sharedFx.zip -d $tmpDir | ||
mkdir -p $output_dir | ||
echo "Copying to $output_dir" | ||
cp $tmpDir/runtimes/win-x64/lib/$framework/* $output_dir | ||
cp $tmpDir/runtimes/win-x64/native/* $output_dir |
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.
Nice to have: If you only need one shared framework and it's the 64-bit one, suggest using 3 jobs instead of two: Build the x64 shared runtime, upload pipeline artifacts you need, then depend on that build in the two Helix jobs. Might also be able to use the x64 shared runtime job as a dependency for the Windows x64 / x86 job. (Question: Will we run any tests on x86 Helix agents or using the x86 Helix shared framework?)
Otherwise, comments above apply here too.
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.
Is there a reason we aren't doing that today with the templates tests building their own shared framework?
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.
Not really. But, we're getting to the point where building the same Windows bits all over the place is just wasting time. Storage is the same because the packages will become artifacts in any case and the network bandwidth to download things isn't that high. My only real question is whether it's worth uploading the artifacts/bin/ folder as a new Windows-only pipeline artifact for use in the Windows Test job.
One slight complication: We don't create package artifacts for public builds from source. Use pipeline artifacts in that case.
As I said, this is nice to have. We can deal w/ the 5 or so Windows x64 builds separately if you prefer.