-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Get Helix ready for servicing #25477
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
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 |
|---|---|---|
|
|
@@ -16,10 +16,6 @@ | |
| <HelixPreCommand Include="call RunPowershell.cmd InstallNode.ps1 $(NodeVersion) %25HELIX_CORRELATION_PAYLOAD%25\node\bin || exit /b 1" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(IsHelixJob)' == 'true' AND '$(TestDependsOnAspNetRef)' == 'true' AND '$(IsTargetingPackBuilding)' == 'true'"> | ||
| <HelixContent Include="$(RepoRoot)artifacts\packages\Release\Shipping\Microsoft.AspNetCore.App.Ref.$(AppRuntimeVersion).nupkg" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition="'$(IsHelixJob)' == 'true' AND '$(TestDependsOnAspNetRuntime)' == 'true'"> | ||
| <HelixContent Include="$(RepoRoot)artifacts\packages\Release\Shipping\*-ci.nupkg" /> | ||
| </ItemGroup> | ||
|
|
@@ -91,11 +87,23 @@ Usage: dotnet msbuild /t:Helix src/MyTestProject.csproj | |
| </Target> | ||
|
|
||
| <Target Name="_CreateHelixWorkItem" Condition="$(BuildHelixPayload)"> | ||
|
|
||
| <ItemGroup> | ||
| <HelixContent Include="$(OutputPath)/Microsoft.VisualStudio.TestPlatform.Extension.Xunit.Xml.TestAdapter.dll" /> | ||
| <HelixContent Include="$(OutputPath)/Microsoft.VisualStudio.TestPlatform.Extension.Xunit.Xml.TestLogger.dll" /> | ||
| <HelixContent Condition="'$(TestDependsOnAspNetRuntime)' == 'true'" Include="$(RepoRoot)artifacts\packages\Release\Shipping\Microsoft.AspNetCore.App.Runtime.win-x64.$(AppRuntimeVersion).nupkg" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- | ||
| Could use _GetPackageVersionInfo target (defined in eng/targets/Packaging.targets and included in every C# | ||
| and F# project) instead of the $(SharedFxVersion) property but the property works everywhere except in site | ||
| extensions projects. Could also use $(TargetingPackVersion) but that has slightly more complicated semantics | ||
| and doesn't keep up when in servicing. | ||
| --> | ||
| <ItemGroup Condition=" '$(TestDependsOnAspNetRef)' == 'true' AND '$(IsTargetingPackBuilding)' == 'true' "> | ||
| <HelixContent Include="$(RepoRoot)artifacts\packages\Release\Shipping\Microsoft.AspNetCore.App.Ref.$(SharedFxVersion).nupkg" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition=" '$(TestDependsOnAspNetRuntime)' == 'true' "> | ||
| <HelixContent Include="$(RepoRoot)artifacts\packages\Release\Shipping\Microsoft.AspNetCore.App.Runtime.win-x64.$(SharedFxVersion).nupkg" /> | ||
| </ItemGroup> | ||
|
|
||
| <PropertyGroup> | ||
|
|
@@ -117,8 +125,8 @@ Usage: dotnet msbuild /t:Helix src/MyTestProject.csproj | |
| <TestAssembly>$(TargetFileName)</TestAssembly> | ||
| <PreCommands>@(HelixPreCommand)</PreCommands> | ||
| <PostCommands>@(HelixPostCommand)</PostCommands> | ||
| <Command Condition="$(IsWindowsHelixQueue)">call runtests.cmd $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(AppRuntimeVersion).nupkg Microsoft.AspNetCore.App.Ref.$(AppRuntimeVersion).nupkg $(HelixTimeout)</Command> | ||
| <Command Condition="!$(IsWindowsHelixQueue)">./runtests.sh $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(AppRuntimeVersion).nupkg Microsoft.AspNetCore.App.Ref.$(AppRuntimeVersion).nupkg $(HelixTimeout)</Command> | ||
| <Command Condition="$(IsWindowsHelixQueue)">call runtests.cmd $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(SharedFxVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(SharedFxVersion).nupkg Microsoft.AspNetCore.App.Ref.$(SharedFxVersion).nupkg $(HelixTimeout)</Command> | ||
|
Member
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. Seems worth considering calculating the runtime from the sharedfxversion, would save us one parameter...
Contributor
Author
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. Maybe. I was originally thinking of parsing the filename i.e. going the other way but dropped that before thinking of creating the filename. But, worthwhile or not, I'd rather get this in if it passes the CI. I'm also unlikely to do the relatively minor follow up but feel free❕ |
||
| <Command Condition="!$(IsWindowsHelixQueue)">./runtests.sh $(TargetFileName) $(NETCoreSdkVersion) $(MicrosoftNETCoreAppRuntimeVersion) $(SharedFxVersion) $(_HelixFriendlyNameTargetQueue) $(TargetArchitecture) $(RunQuarantinedTests) $(DotnetEfPackageVersion) Microsoft.AspNetCore.App.Runtime.win-x64.$(SharedFxVersion).nupkg Microsoft.AspNetCore.App.Ref.$(SharedFxVersion).nupkg $(HelixTimeout)</Command> | ||
| <Command Condition="$(HelixCommand) != ''">$(HelixCommand)</Command> | ||
| <Timeout>$(HelixTimeout)</Timeout> | ||
| </HelixWorkItem> | ||
|
|
||
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.
I also wonder if this hard-code version was causing problems when using
eng/scripts/RunHelix.ps1locally, where the version would be5.0.0-deveven now❔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.
@HaoK what do you thunk❔
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.
Yes for sure, that's what we use for the directory we install to, if you were seeing weirdness with the bits not installed correctly this would be why