-
Notifications
You must be signed in to change notification settings - Fork 543
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
[tests] Add support for running EndToEnd tests on helix with the aspire
workload
#2534
Changes from all commits
9770c7d
f62530d
99f6a42
bf1bb7d
116f4d0
ce61861
39472c8
949294f
ef410ff
9aaf1ec
8649005
a976089
94a50c4
56c4f2d
800ed52
28430fd
e5afe1b
a2116d8
a57a478
149b6ff
2705b8c
c1b2b68
4199746
c6b9e56
3f8cd92
af1e0de
8dfe316
8072559
548a340
94e1d29
57bc07b
5be48e0
a600106
ffa8b33
8434d15
d951135
64a9eb7
e3e1a65
f49215a
bdb4081
0e79ba0
59085f9
5e4943b
aff6eb7
02f5cf0
4a0ea34
c40da44
b390a0c
56b7c61
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 |
---|---|---|
|
@@ -10,8 +10,6 @@ | |
<VSTemp>$(WorkloadIntermediateOutputPath)VS/</VSTemp> | ||
<WorkloadOutputPath>$(ArtifactsBinDir)workloads/</WorkloadOutputPath> | ||
<WorkloadOutputPath Condition="'$(workloadArtifactsPath)' != ''">$(workloadArtifactsPath)/</WorkloadOutputPath> | ||
<PackageSource>$(ArtifactsShippingPackagesDir)</PackageSource> | ||
<PackageSource Condition="'$(workloadPackagesPath)' != ''">$(workloadPackagesPath)/</PackageSource> | ||
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. Does this mean that this cannot get overriden any longer? cc: @danegsta in case he is aware if we depend on this. 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. Where does 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. Should be fine as long as the workload still builds correctly |
||
<RunAnalyzers>false</RunAnalyzers> | ||
</PropertyGroup> | ||
|
||
|
@@ -75,11 +73,11 @@ | |
</ShortNames> | ||
</ItemGroup> | ||
|
||
<MSBuild Projects="../dashboardpack/dashboardpack.csproj" Targets="restore;build" Properties="DashboardRuntime=%(_PackRuntimes.Identity);PackageOutputPath=$(PackageSource)" /> | ||
<MSBuild Projects="../dcppack/dcppack.csproj" Targets="restore;build" Properties="DcpRuntime=%(_PackRuntimes.Identity);PackageOutputPath=$(PackageSource)" /> | ||
<MSBuild Projects="../dashboardpack/dashboardpack.csproj" Targets="restore;build" Properties="DashboardRuntime=%(_PackRuntimes.Identity);PackageOutputPath=$(WorkloadsPackageSource)" /> | ||
<MSBuild Projects="../dcppack/dcppack.csproj" Targets="restore;build" Properties="DcpRuntime=%(_PackRuntimes.Identity);PackageOutputPath=$(WorkloadsPackageSource)" /> | ||
|
||
<ItemGroup> | ||
<ManifestPackages Include="$(PackageSource)Microsoft.NET.Sdk.Aspire.Manifest*.nupkg" | ||
<ManifestPackages Include="$(WorkloadsPackageSource)Microsoft.NET.Sdk.Aspire.Manifest*.nupkg" | ||
MsiVersion="$(MsiVersion)" | ||
SupportsMachineArch="true" /> | ||
</ItemGroup> | ||
|
@@ -89,7 +87,7 @@ | |
BaseOutputPath="$(WorkloadOutputPath)" | ||
EnableSideBySideManifests="true" | ||
ComponentResources="@(ComponentResources)" | ||
PackageSource="$(PackageSource)" | ||
PackageSource="$(WorkloadsPackageSource)" | ||
ShortNames="@(ShortNames)" | ||
WorkloadManifestPackageFiles="@(ManifestPackages)" | ||
WixToolsetPath="$(WixToolsetPath)" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<Project> | ||
<PropertyGroup> | ||
<VersionBand Condition=" '$(VersionBand)' == '' ">$([System.Text.RegularExpressions.Regex]::Match($(MicrosoftDotnetSdkInternalPackageVersion), `^\d+\.\d+\.\d`))00</VersionBand> | ||
<!-- When we are ready to produce Workloads targeting the stable SDK band, set UseStableSdkBand to true. Otherwise, it will match the SDK preview band. --> | ||
<UseStableSdkBand>true</UseStableSdkBand> | ||
<DotNetVersionBand Condition=" '$(UseStableSdkBand)' != 'true' ">$(VersionBand)$([System.Text.RegularExpressions.Regex]::Match($(MicrosoftDotnetSdkInternalPackageVersion), `\-(preview|rc|alpha|rtm).\d+`))</DotNetVersionBand> | ||
<DotNetVersionBand Condition="'$(UseStableSdkBand)' == 'true'">$(VersionBand)</DotNetVersionBand> | ||
<DotNetAspireManifestVersionBand>$(DotNetVersionBand)</DotNetAspireManifestVersionBand> | ||
</PropertyGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
<Project> | ||
<Import Project="Workload.props" /> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<PackageType>DotnetPlatform</PackageType> | ||
|
@@ -12,13 +14,7 @@ | |
<DotNetOutputPath>$(RepoRoot)artifacts/bin/</DotNetOutputPath> | ||
<DotNetDirectory>$(DotNetOutputPath)dotnet/</DotNetDirectory> | ||
<DotNetPacksDirectory>$(DotNetDirectory)packs/</DotNetPacksDirectory> | ||
<VersionBand Condition=" '$(VersionBand)' == '' ">$([System.Text.RegularExpressions.Regex]::Match($(MicrosoftDotnetSdkInternalPackageVersion), `^\d+\.\d+\.\d`))00</VersionBand> | ||
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. Just curious, why extract only those properties and not the rest? 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 extracted properties define the version, and band used for the workload. The remaining properties are relevant to the various projects that import it for building packages. Some of these common properties could still be moved to the props file, but not all. |
||
<!-- When we are ready to produce Workloads targeting the stable SDK band, set UseStableSdkBand to true. Otherwise, it will match the SDK preview band. --> | ||
<UseStableSdkBand>true</UseStableSdkBand> | ||
<DotNetVersionBand Condition=" '$(UseStableSdkBand)' != 'true' ">$(VersionBand)$([System.Text.RegularExpressions.Regex]::Match($(MicrosoftDotnetSdkInternalPackageVersion), `\-(preview|rc|alpha|rtm).\d+`))</DotNetVersionBand> | ||
<DotNetVersionBand Condition="'$(UseStableSdkBand)' == 'true'">$(VersionBand)</DotNetVersionBand> | ||
<DotNetSdkManifestsFolder>$(DotNetVersionBand)</DotNetSdkManifestsFolder> | ||
<DotNetAspireManifestVersionBand>$(DotNetVersionBand)</DotNetAspireManifestVersionBand> | ||
<_AspireDotNetVersionMajor>8</_AspireDotNetVersionMajor> | ||
<_AspireDotNetVersionMinor>0</_AspireDotNetVersionMinor> | ||
<_AspireDotNetVersion>$(_AspireDotNetVersionMajor).$(_AspireDotNetVersionMinor)</_AspireDotNetVersion> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<RunSettings> | ||
<RunConfiguration> | ||
<!-- Timeout in ms, 15 minutes --> | ||
<TestSessionTimeout>900000</TestSessionTimeout> | ||
<!-- Filter out failing (wrong framework, platform, runtime or activeissue) tests --> | ||
<TestCaseFilter>category!=failing</TestCaseFilter> | ||
</RunConfiguration> | ||
<LoggerRunSettings> | ||
<Loggers> | ||
<Logger friendlyName="trx"> | ||
<Configuration> | ||
<LogFileName>TestResults.trx</LogFileName> | ||
</Configuration> | ||
</Logger> | ||
<Logger friendlyName="console"> | ||
<Configuration> | ||
<Verbosity>normal</Verbosity> | ||
</Configuration> | ||
</Logger> | ||
</Loggers> | ||
</LoggerRunSettings> | ||
</RunSettings> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,31 @@ | |
|
||
<PropertyGroup> | ||
<TargetFramework>$(NetCurrent)</TargetFramework> | ||
|
||
<!-- no docker support on helix/windows yet --> | ||
<RunTestsOnHelix Condition="'$(OS)' != 'Windows_NT'">true</RunTestsOnHelix> | ||
<SkipTests Condition="'$(OS)' == 'Windows_NT'">true</SkipTests> | ||
Comment on lines
+6
to
+8
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. Can we just say 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 we can do it like that also. The reason for "also" is that if we don't have any tests that would get run, then not skipping the whole thing would mean:
I think we can add the attributes also, and for the specific case (windows) where nothing can run, we skip in the project file too. Thoughts? 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. How long until we are able to run these tests on Windows? |
||
|
||
<!-- no docker support on helix/windows yet --> | ||
<TestUsingWorkloads Condition="! ('$(ContinuousIntegrationBuild)' == 'true' and '$(OS)' == 'Windows_NT')">true</TestUsingWorkloads> | ||
<InstallWorkloadForTesting>$(TestUsingWorkloads)</InstallWorkloadForTesting> | ||
|
||
<BuiltNuGetsDir>$(ArtifactsShippingPackagesDir)</BuiltNuGetsDir> | ||
<PackageVersionForWorkloadManifests>$(PackageVersion)</PackageVersionForWorkloadManifests> | ||
<DefineConstants Condition="'$(TestsRunningOutsideOfRepo)' == 'true'">TESTS_RUNNING_OUTSIDE_OF_REPO;$(DefineConstants)</DefineConstants> | ||
|
||
<XunitRunnerJson>xunit.runner.json</XunitRunnerJson> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<Compile Include="..\testproject\Common\TestResourceNames.cs" /> | ||
<Compile Include="..\Shared\WorkloadTesting\*.cs" Link="WorkloadTestingCommon" /> | ||
|
||
<None Include="..\testproject\**\*" Link="testassets\testproject\%(RecursiveDir)%(FileName)%(Extension)" CopyToOutputDirectory="PreserveNewest" /> | ||
<None Include="$(PatchedNuGetConfigPath)" Link="testassets\testproject\nuget.config" CopyToOutputDirectory="PreserveNewest" /> | ||
<None Include="$(RepoRoot)Directory.Packages.props" Link="testassets\testproject\Directory.Packages.repo.props" CopyToOutputDirectory="PreserveNewest" /> | ||
|
||
<PackageReference Include="Microsoft.Extensions.Http.Resilience" /> | ||
<PackageReference Include="Microsoft.DotNet.XUnitExtensions" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<Project> | ||
<PropertyGroup> | ||
<IsHelixOnlyTestProject>true</IsHelixOnlyTestProject> | ||
<IsWorkloadTestProject>true</IsWorkloadTestProject> | ||
<RunSettingsFilePath>$(MSBuildThisFileDirectory).runsettings</RunSettingsFilePath> | ||
</PropertyGroup> | ||
|
||
<Import Project="..\Directory.Build.props" /> | ||
</Project> |
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.
Where is this package source defined?
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.
https://github.com/dotnet/runtime/blob/f135699310bccfa83b3b441d2f4f1571fe8df480/src/tasks/WorkloadBuildTasks/InstallWorkloadFromArtifacts.cs#L262
I agree that it does seem odd that you can't directly see what this links up to in the repo. I can have the package source name be passed to the task, which would be in aspire repo.
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.
Does this throw a warning at all on the initial restore before the task has a chance to replace the source? Did we also consider manually passing in
-s <path>
to a manual invocation of restore instead of having this in the config?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.
No warnings. Wouldn't
-s path
only add a package source but not the mapping, and because of that restoring from the source would fail. That is what I hit when I removed this mapping.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.
Once dotnet/runtime#99277 is merged this entry can be removed, and it would be added at build time.