Upgrade tooling for 9.0x #5483

radical · 2024-08-29T23:07:46Z

Update dependencies from .NET 9 Eng channel
Update nuget.config to use dotnet9 nuget feed instead of nuget8
Update tests to run with 8.0 runtime
Resolve new xunit analyzer warnings

All the projects still target net8.0.

Future TODO:

add 9.0 templates, and tests

Fixes #5446 .

TODO before merge:

check that code coverage report is unaffected
Number of tests is the same as a main run
Diff package build output to ensure contents of packages are unaffected (added by @joperezr)
Validate with an internal build

Microsoft Reviewers: Open in CodeFlow

src/Aspire.Dashboard/Otlp/Storage/ApplicationKey.cs

tests/Aspire.Hosting.Testing.Tests/TestingBuilderTests.cs

joperezr · 2024-08-30T23:13:25Z

tests/Aspire.Workload.Tests/StarterTemplateRunTestsBase.cs

@@ -103,7 +103,7 @@ static async Task<List<string[]>> GetAndValidateCellTexts(ILocator tableLoc)
                    r => Assert.True(DateTime.TryParse(r, out _)),
                    r => Assert.True(int.TryParse(r, out var actualTempC) && actualTempC >= -20 && actualTempC <= 55),
                    r => Assert.True(int.TryParse(r, out var actualTempF) && actualTempF >= -5 && actualTempF <= 133),
-                    r => Assert.Contains(r, ["Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"]));
+                    r => Assert.Contains(r, new HashSet<string>(){"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"}));


Interesting, I would have expected this to work with the new collection literals.

IIRC it was not being able to resolve the overload between HashSet<> and SortedSet<>.

joperezr · 2024-08-30T23:16:26Z

tests/Shared/WorkloadTesting/data/nuget8.config

@@ -9,7 +9,7 @@
    <add key="built-local" value="%BUILT_NUGETS_PATH%" />
    <add key="dotnet-public" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-public/nuget/v3/index.json" />
    <add key="dotnet-eng" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-eng/nuget/v3/index.json" />
-    <add key="dotnet8" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet8/nuget/v3/index.json" />
+    <add key="dotnet9" value="https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet9/nuget/v3/index.json" />


So we don't need dotnet8 feed anymore? I think we still push our packages there so we have to update our default channels in darc.

But we shouldn't be fetching any of our packages from these feeds (except BUILT_NUGETS_PATH) for tests. Or am I missing something?

joperezr · 2024-08-30T23:19:47Z

tests/Shared/Aspire.Workload.Testing.targets

@@ -19,6 +19,9 @@

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Runtime.WorkloadTesting.Internal" />
+
+    <AdditionalSharedFrameworkToInstallArguments Include="-Runtime dotnet -Version 8.0.8 -Channel 8.0" />


Is there a way to reuse the dotnet installed by the original build.cmd/sh call so we don't have to add this? I want to avoid needing to update a number in several places (runtime framework in this case 8.0.8) as those change each month.

This is specifically for workload testing. It will be much cleaner to use the existing support for this. But we could try to use the restored .dotnet also, but also need to account for .dotnet not being installed when the system dotnet is sufficient.

joperezr · 2024-08-30T23:20:51Z

tests/helix/send-to-helix-inner.proj

@@ -8,13 +8,15 @@

    <IncludeDotNetCli>true</IncludeDotNetCli>
    <DotNetCliPackageType>sdk</DotNetCliPackageType>
+    <DotNetCliVersion>8.0.401</DotNetCliVersion>


How will this work for template tests for 9.0 templates? won't they require a 9.0 SDK?

Still going through a first pass to get tests passing :D

This will require us to bundle the sdk ourselves, because arcade doesn't seem to support additional runtimes.
To confirm that this works I think this PR should include a couple of tests where the test project targets net9.0 (best in workload tests).

joperezr · 2024-08-30T23:22:27Z

Directory.Build.props

@@ -3,6 +3,7 @@
  <Import Project="$(RepositoryEngineeringDir)/apicompat/PublicApiAnalyzer.props" />

  <PropertyGroup>
+    <NetCurrent>net8.0</NetCurrent>


Is this property used anywhere else in arcade or is it solely used by consuming repos? my concern is that changing this in order to avoid having to modify individual projects might unintentionally affect how the infrastructure work.

Good point. I can drop this in the cleanup.

Looks like efcore calls this DefaultNetCoreTargetFramework.

https://github.com/dotnet/efcore/blob/ecfee78eb1fa2b2eaa0dbf945f1d4f8fa571be74/src/EFCore.Abstractions/EFCore.Abstractions.csproj#L5C24-L5C53

global.json

src/Tools/ConfigurationSchemaGenerator/ConfigurationSchemaGenerator.csproj

radical · 2024-09-04T21:40:05Z

/azp run

azure-pipelines · 2024-09-04T21:40:20Z

Azure Pipelines successfully started running 1 pipeline(s).

Updating 'Microsoft.SourceBuild.Intermediate.source-build-reference-packages': '8.0.0-alpha.1.23516.4' => '9.0.0-alpha.1.24453.2' (from build '20240903.2' of 'https://github.com/dotnet/source-build-reference-packages') Updating 'Microsoft.SourceBuild.Intermediate.arcade': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.Arcade.Sdk': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.Build.Tasks.Installers': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.Build.Tasks.Workloads': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.Helix.Sdk': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.RemoteExecutor': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.SharedFramework.Sdk': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade') Updating 'Microsoft.DotNet.XUnitExtensions': '8.0.0-beta.24426.2' => '9.0.0-beta.24453.1' (from build '20240903.1' of 'https://github.com/dotnet/arcade')

`Do not use Assert.Equal() to check for collection size. Use Assert.Single instead.xUnit2013`

`Do not use Assert.Empty() to check if a value does not exist in a collection. Use Assert.DoesNotContain() instead.xUnit2029`

`error xUnit1030: Test methods should not call ConfigureAwait(false), as it may bypass parallelization limits. Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007. (https://xunit.net/xunit.analyzers/rules/xUnit1030)`

Warning of the form: `error xUnit1012: Null should not be used for type parameter 'secondaryApiKey' of type 'string'. Use a non-null value, or convert the parameter to a nullable type. (https://xunit.net/xunit.analyzers/rules/xUnit1012)`

`error CS0121: The call is ambiguous between the following methods or properties: 'Assert.Contains<T>(T, HashSet<T>)' and 'Assert.Contains<T>(T, SortedSet<T>)'`

radical · 2024-09-05T06:02:15Z

TODO before merge:

check that code coverage report is unaffected
Number of tests is the same as a main run
Diff package build output to ensure contents of packages are unaffected (added by @joperezr)

radical · 2024-09-05T19:21:54Z

eng/Version.Details.xml

-    <!-- Intermediate is necessary for source build. -->
-    <Dependency Name="Microsoft.SourceBuild.Intermediate.xliff-tasks" Version="1.0.0-beta.23475.1" CoherentParentDependency="Microsoft.DotNet.Arcade.Sdk">
-      <Uri>https://github.com/dotnet/xliff-tasks</Uri>
-      <Sha>73f0850939d96131c28cf6ea6ee5aacb4da0083a</Sha>
-      <SourceBuild RepoName="xliff-tasks" ManagedOnly="true" />
-    </Dependency>


From the first commit - does dropping this seem correct? The assembly has moved to dotnet/arcade.

I think that is fine, and I think you can also drop the dependency over from lines 152-155 above for that matter (not sure why we had both), but shouldn't we have now a new dependency coming from arcade for it?

We'll need to add a dependency like dotnet/runtime has:

<Dependency Name="Microsoft.DotNet.XliffTasks" Version="9.0.0-beta.24423.2"> <Uri>https://github.com/dotnet/arcade</Uri> <Sha>91599268652b51969b8d8088d4f2f2ba7b3ebb19</Sha> </Dependency>

added that here.

radical · 2024-09-05T19:22:58Z

Read commit by commit to review this. The first commit is the biggest and is the update of the dependencies, mostly from arcade.

radical · 2024-09-09T19:32:40Z

/azp run

azure-pipelines · 2024-09-09T19:32:55Z

Azure Pipelines successfully started running 1 pipeline(s).

# Conflicts: # tests/Aspire.Elastic.Clients.Elasticsearch.Tests/Aspire.Elastic.Clients.Elasticsearch.Tests.csproj

…(38,45): error CA2007: Consider calling ConfigureAwait on the awaited task

joperezr · 2024-09-10T22:12:41Z

eng/Versions.props

@@ -12,6 +12,12 @@
      This should get updated each time packages are shipped.
    -->
    <BaselineVersionForPackageValidation>8.0.1</BaselineVersionForPackageValidation>
+    <DefaultTargetFramework>net8.0</DefaultTargetFramework>
+    <!-- dotnet 8.0 versions for running tests -->
+    <DotNetRuntimeVersionForTesting>8.0.8</DotNetRuntimeVersionForTesting>


I'm wondering how we can keep this one up-to-date. One option would be if we use the Microsoft.NetCore.App.Ref package and dependency flow for the version as we know that one will push the update to us. Anyway, no need to fix it now but we should consider how to update this (and the sdk below)

dotnet/runtime is using Microsoft.DotNet.ApiCompat.Task for that. Either should work well.

joperezr · 2024-09-10T22:23:44Z

tests/Aspire.Workload.Tests/StarterTemplateRunTestsBase.cs

@@ -103,7 +103,7 @@ static async Task<List<string[]>> GetAndValidateCellTexts(ILocator tableLoc)
                    r => Assert.True(DateTime.TryParse(r, out _)),
                    r => Assert.True(int.TryParse(r, out var actualTempC) && actualTempC >= -20 && actualTempC <= 55),
                    r => Assert.True(int.TryParse(r, out var actualTempF) && actualTempF >= -5 && actualTempF <= 133),
-                    r => Assert.Contains(r, ["Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"]));
+                    r => Assert.Contains(r, new HashSet<string>{"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"}));


Still curious why this didn't work with the collection initializer, but oh well, no need to change it.

error CS0121: The call is ambiguous between the following methods or properties: 'Assert.Contains<T>(T, HashSet<T>)' and 'Assert.Contains<T>(T, SortedSet<T>)'

joperezr · 2024-09-10T22:28:21Z

tests/helix/send-to-helix-inner.proj

@@ -13,8 +13,7 @@
    <!-- Disable playwright tests on helix/linux. Issue: https://github.com/dotnet/aspire/issues/4623 -->
    <DisablePlaywrightTests Condition="'$(OS)' != 'Windows_NT'">true</DisablePlaywrightTests>

-    <GlobalJsonContent>$([System.IO.File]::ReadAllText('$(RepoRoot)global.json'))</GlobalJsonContent>
-    <DotNetCliVersion>$([System.Text.RegularExpressions.Regex]::Match($(GlobalJsonContent), '(%3F&lt;="dotnet": ").*(%3F=")'))</DotNetCliVersion>
+    <DotNetCliVersion>$(DotNetSdkVersionForTesting)</DotNetCliVersion>


We'll want to make sure now that we update that property so that it at least equals the one in global.json. I wonder if we can add a target somewhere in our build that detects if we've fallen behind and if so to fail the build...

No need to add that in this change though, don't block this PR for that.

Once we get the version from a dotnet/sdk dependency, it should always(?) be newer than global.json.

joperezr · 2024-09-10T22:29:03Z

global.json

@@ -1,16 +1,30 @@
 {
  "sdk": {
-    "version": "8.0.303",
+    "version": "9.0.100-preview.7.24407.12",


This is fine for now. We'll want to bump to rc1 that got shipped today soon.

joperezr

Changes look great @radical thanks for working on this! Let's prioritize getting this merged given the breath as otherwise it is likely to get conflicts with other PRs.

radical · 2024-09-10T22:48:25Z

Issue to track follow up work - #5676

radical · 2024-09-10T22:53:49Z

Internal build for validation - https://dev.azure.com/dnceng/internal/_build/results?buildId=2535499

.. in workload testing as it is required for the internal build. `Aspire.Hosting.Sdk.Msi.arm64.9.0.0-preview.4.24460.2.nupkg, Aspire.Hosting.Sdk.Msi.x64.9.0.0-preview.4.24460.2.nupkg, Aspire.Hosting.Sdk.Msi.x86.9.0.0-preview.4.24460.2.nupkg, Aspire.ProjectTemplates.Msi.arm64.9.0.0-preview.4.24460.2.nupkg, Aspire.ProjectTemplates.Msi.x64.9.0.0-preview.4.24460.2.nupkg, Aspire.ProjectTemplates.Msi.x86.9.0.0-preview.4.24460.2.nupkg`

…ests .. like the various hosting integration tests. The run time can vary especially if docker needs to fetch an image.

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Aug 29, 2024

radical added area-engineering-systems and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2024

radical force-pushed the switch-9.0 branch from 30cfd4d to 692521f Compare August 29, 2024 23:38

radical mentioned this pull request Aug 30, 2024

Update Arcade and Tooling on the repo to use 9.0 #5446

Closed

build-analysis bot mentioned this pull request Aug 30, 2024

Some tests timing out on helix #5247

Open

joperezr reviewed Aug 30, 2024

View reviewed changes

src/Aspire.Dashboard/Otlp/Storage/ApplicationKey.cs Outdated Show resolved Hide resolved

joperezr reviewed Aug 30, 2024

View reviewed changes

tests/Aspire.Hosting.Testing.Tests/TestingBuilderTests.cs Outdated Show resolved Hide resolved

joperezr reviewed Aug 30, 2024

View reviewed changes

eerhardt reviewed Sep 3, 2024

View reviewed changes

global.json Outdated Show resolved Hide resolved

eerhardt reviewed Sep 3, 2024

View reviewed changes

src/Tools/ConfigurationSchemaGenerator/ConfigurationSchemaGenerator.csproj Outdated Show resolved Hide resolved

radical force-pushed the switch-9.0 branch from bcd65f1 to f320639 Compare September 4, 2024 19:54

radical added 12 commits September 4, 2024 22:59

Add dotnet 8 versions for testing

6225452

[tests] Install 8.0 runtime for workload testing

eb1c382

Resolve xUnit2013 warning

d70d1bb

`Do not use Assert.Equal() to check for collection size. Use Assert.Single instead.xUnit2013`

Resolve xUnit2029 warning

4f316c8

`Do not use Assert.Empty() to check if a value does not exist in a collection. Use Assert.DoesNotContain() instead.xUnit2029`

Track API change in EqualException

20d94c5

Ignore warning xUnit1030

a152212

`error xUnit1030: Test methods should not call ConfigureAwait(false), as it may bypass parallelization limits. Omit ConfigureAwait, or use ConfigureAwait(true) to avoid CA2007. (https://xunit.net/xunit.analyzers/rules/xUnit1030)`

Fix xUnit1012 analyzer warning.

762f5ea

Warning of the form: `error xUnit1012: Null should not be used for type parameter 'secondaryApiKey' of type 'string'. Use a non-null value, or convert the parameter to a nullable type. (https://xunit.net/xunit.analyzers/rules/xUnit1012)`

Fix compile error.

48f233b

`error CS0121: The call is ambiguous between the following methods or properties: 'Assert.Contains<T>(T, HashSet<T>)' and 'Assert.Contains<T>(T, SortedSet<T>)'`

couple more fixes

af944fe

Update nuget.config to use dotnet9 sources instead of dotnet8

f411371

Remove unnecessary package reference to Microsoft.DotNet.XunitAssert

9d559e2

cleanup

d27a6cf

radical force-pushed the switch-9.0 branch from 62e1870 to d27a6cf Compare September 5, 2024 03:09

radical marked this pull request as ready for review September 5, 2024 04:42

radical requested review from mitchdenny, ReubenBond and sebastienros as code owners September 5, 2024 04:42

radical commented Sep 5, 2024

View reviewed changes

radical added 4 commits September 10, 2024 01:57

Merge remote-tracking branch 'origin/main' into switch-9.0

b204aac

# Conflicts: # tests/Aspire.Elastic.Clients.Elasticsearch.Tests/Aspire.Elastic.Clients.Elasticsearch.Tests.csproj

Fix AzureFunctionsEndToEnd

2a94c7c

Track changes for Aspire.Hosting.Azure.Functions

c15d3c4

Fix CA2007: src\Aspire.Hosting\Health\ResourceHealthCheckScheduler.cs…

239c049

…(38,45): error CA2007: Consider calling ConfigureAwait on the awaited task

joperezr reviewed Sep 10, 2024

View reviewed changes

joperezr approved these changes Sep 10, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into switch-9.0

e4bb2a6

radical mentioned this pull request Sep 10, 2024

Follow up work for upgrade tooling to 9.0x #5676

Open

3 tasks

Add dependency to Microsoft.DotNet.XliffTasks from dotnet/arcade

4158dcf

radical added 2 commits September 10, 2024 19:28

[tests] Increase the default timeout from 10mins to 15mins for basict…

6a182aa

…ests .. like the various hosting integration tests. The run time can vary especially if docker needs to fetch an image.

radical merged commit b4fc095 into dotnet:main Sep 11, 2024
11 checks passed

radical deleted the switch-9.0 branch September 11, 2024 18:10

github-actions bot locked and limited conversation to collaborators Oct 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade tooling for 9.0x #5483

Upgrade tooling for 9.0x #5483

radical commented Aug 29, 2024 •

edited

Loading

joperezr Aug 30, 2024

radical Aug 30, 2024

joperezr Aug 30, 2024

radical Sep 4, 2024

joperezr Aug 30, 2024

radical Aug 30, 2024

joperezr Aug 30, 2024

radical Aug 30, 2024

radical Aug 30, 2024

joperezr Aug 30, 2024

radical Aug 30, 2024

eerhardt Sep 3, 2024

radical commented Sep 4, 2024

azure-pipelines bot commented Sep 4, 2024

radical commented Sep 5, 2024 •

edited by joperezr

Loading

radical Sep 5, 2024

joperezr Sep 10, 2024

radical Sep 10, 2024

radical Sep 10, 2024

radical commented Sep 5, 2024

radical commented Sep 9, 2024

azure-pipelines bot commented Sep 9, 2024

joperezr Sep 10, 2024

radical Sep 10, 2024

joperezr Sep 10, 2024

radical Sep 10, 2024

joperezr Sep 10, 2024

radical Sep 10, 2024

joperezr Sep 10, 2024

joperezr left a comment

radical commented Sep 10, 2024

radical commented Sep 10, 2024

Upgrade tooling for 9.0x #5483

Upgrade tooling for 9.0x #5483

Conversation

radical commented Aug 29, 2024 • edited Loading

Microsoft Reviewers: Open in CodeFlow

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radical commented Sep 4, 2024

azure-pipelines bot commented Sep 4, 2024

radical commented Sep 5, 2024 • edited by joperezr Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

radical commented Sep 5, 2024

radical commented Sep 9, 2024

azure-pipelines bot commented Sep 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joperezr left a comment

Choose a reason for hiding this comment

radical commented Sep 10, 2024

radical commented Sep 10, 2024

radical commented Aug 29, 2024 •

edited

Loading

radical commented Sep 5, 2024 •

edited by joperezr

Loading