Skip to content
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

Enabling .NET 9 tests for SF SDK packages #390

Merged
merged 13 commits into from
Oct 9, 2024

Conversation

plave0
Copy link
Contributor

@plave0 plave0 commented Oct 1, 2024

This PR enables tests for .NET 9 builds.

Compatibility with the deprecated BinaryFormatter has been introduced so that .NET 9 tests can be enabled.

Additionally, we are moving away from the buils.ps1 script, which uses msbuild under the hood. Instead, we are moving to the dotnet tool, in the aim of streamlining the build process and infrastructure.

Internal ADO work item: https://dev.azure.com/msazure/One/_workitems/edit/29603235

@plave0
Copy link
Contributor Author

plave0 commented Oct 1, 2024

@olegsych, these are the changes I made for now.

When running dotnet test .\code.sln --no-build -v q, I get the following warnings:

C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Microsoft.TestPlatform.targets(48,5): warning : [xUnit.net 00:00:00.10] Skipping: Microsoft.ServiceFabric.Services.Tests (could not find dependent assembly 'System.Runtime, Version=9.0.0')
C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Microsoft.TestPlatform.targets(48,5): warning : No test is available in Q:\source\service-fabric-services-and-actors-dotnet\test\unittests\Microsoft.ServiceFabric.Services.Tests\bin\Debug\Microsoft.ServiceFabric.Services.Tests.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.
C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Microsoft.TestPlatform.targets(48,5): warning : [xUnit.net 00:00:00.14] Skipping: Microsoft.ServiceFabric.Services.Tests (could not find dependent assembly 'System.Runtime, Version=9.0.0')
C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Microsoft.TestPlatform.targets(48,5): warning : No test is available in Q:\source\service-fabric-services-and-actors-dotnet\test\unittests\Microsoft.ServiceFabric.Services.Tests\bin\Debug\Microsoft.ServiceFabric.Services.Tests.dll. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.

This does not happen for the other suits where I enabled .NET 9 tests. Do this error messages look familiar?

@plave0
Copy link
Contributor Author

plave0 commented Oct 1, 2024

@olegsych, also, Build test is failing due to being unable to build .NET 9 tests, so we will have to change the pipeline after all, in order to include the .NET 9 SDK.

@olegsych
Copy link
Member

olegsych commented Oct 2, 2024

@plave0 I think the solution may not be setup to build both net8.0 and net9.0 binaries at the same time. Try specifying only <TargetFramework>net9.0</TargetFramework> in the test projects and see if the tests run correctly just on .NET 9.

I suspect the problem preventing .NET 9 tests from running in addition to .NET 8 is here: https://github.com/microsoft/service-fabric-services-and-actors-dotnet/blob/develop/properties/service_fabric_common.props#L10. I'm not sure why we are overriding the OutputPath property with what should be its default value. In combination with this https://github.com/microsoft/service-fabric-services-and-actors-dotnet/blob/develop/properties/service_fabric_managed_common.props#L12, we are forcing binaries with different target frameworks to override each other. In a clean new xUnit test project, these properties are not specified and the build output is directed to bin\$(Configuration)\$(TargetFramework). I think we should remove both overrides, use the default build output structure with the extra folder for the target framework.

@olegsych
Copy link
Member

olegsych commented Oct 2, 2024

We seem to be having a lot of scope creep and I want us to have a meaningful result to demo on Monday. So if you can get the tests running on .NET 9, without .NET 8, I think that might be a good stopping point for this PR.

Making the solution build projects with additional folder for target framework could need more changes and we don't have much time left. So I would start that as a separate pull request next.

Reverting to building tests only for .NET 9 to resolve issues when
targeting multiple frameworks.
@plave0
Copy link
Contributor Author

plave0 commented Oct 2, 2024

@olegsych, I agree with your suggestion. I have reverted back to targeting only .NET 9.

plave0 added 5 commits October 2, 2024 10:55
Microsoft.ServiceFabric.Service.Remoting.Tests has been ported to .NET 9
and modified to include BinaryFormatter compatibility package.
Disabling building of tests suits for testing purposes
@plave0 plave0 force-pushed the user/pavleiri/enableDotNet9Tests branch from 3e1aa70 to 2996779 Compare October 2, 2024 12:31
@plave0 plave0 requested review from yashagarwal23 and olegsych and removed request for yashagarwal23 October 2, 2024 13:27
.github/workflows/build.yml Show resolved Hide resolved
buildAll.proj Outdated Show resolved Hide resolved
buildAll.proj Outdated Show resolved Hide resolved
@plave0 plave0 marked this pull request as ready for review October 4, 2024 06:42
@plave0 plave0 requested a review from olegsych October 4, 2024 06:56
@plave0 plave0 enabled auto-merge (squash) October 4, 2024 07:08
We are still unable to use code.sln for building the repo, since it
doesn't include SF.ActorsServices.Internal. We will insted build
buildAll.proj for now.
@plave0 plave0 requested a review from olegsych October 7, 2024 11:03
@plave0 plave0 merged commit 23277e0 into develop Oct 9, 2024
2 checks passed
@plave0 plave0 deleted the user/pavleiri/enableDotNet9Tests branch October 9, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants