csharp icon indicating copy to clipboard operation
csharp copied to clipboard

Tests with netstandard2.0/2.1 version of KubernetesClient without .NET Core 3.1

Open m3nax opened this issue 3 years ago • 13 comments

After a long journey looking for a way to compile test projects by forcing the use of a specific version of a "ProjectReference" I wrote the following pipeline.

Now it is certain that the compiler takes the netstandard2.1 version of the library.

How test the solution:

  • download this branch
  • clean project
  • run: dotnet build -f netstandard2.1 --configuration Debug -v minimal ./src/KubernetesClient/KubernetesClient.csproj
  • run; dotnet build -f netstandard2.0 --configuration Debug -v minimal ./tests/SkipTestLogger/SkipTestLogger.csproj

Now set the flag UseLibNetStandard2_1 to false or omit it

  • run: dotnet build --configuration Debug -v minimal --no-dependencies -p:UseLibNetStandard2_1=false ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj
  • run: dotnet build --configuration Debug -v minimal --no-dependencies -p:UseLibNetStandard2_1=false ./tests/E2E.Tests/E2E.Tests.csproj

Now set the flag UseLibNetStandard2_1 to true

  • run: dotnet build --configuration Debug -v minimal --no-dependencies -p:UseLibNetStandard2_1=true ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj
  • run: dotnet build --configuration Debug -v minimal --no-dependencies -p:UseLibNetStandard2_1=true ./tests/E2E.Tests/E2E.Tests.csproj

It switch between forced and standard inclusion of ProjectReference inside KubernetesClient.Tests.csproj and E2E.Tests.csproj:

<!-- standard -->
<ProjectReference Include="..\..\src\KubernetesClient\KubernetesClient.csproj"/>

<!-- with forced target framework -->
<ProjectReference Include="..\..\src\KubernetesClient\KubernetesClient.csproj" AdditionalProperties="TargetFramework=netstandard2.1"/>

m3nax avatar Sep 22 '22 15:09 m3nax

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: m3nax Once this PR has been reviewed and has the lgtm label, please assign tg123 for approval by writing /assign @tg123 in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Sep 22 '22 15:09 k8s-ci-robot

Related #988

m3nax avatar Sep 22 '22 15:09 m3nax

Not sure if the compiler today takes it here is the thread when i want to solve the issue https://github.com/dotnet/sdk/issues/1791

could you please link me any official doc about the property?

tg123 avatar Sep 22 '22 17:09 tg123

Not sure if the compiler today takes it here is the thread when i want to solve the issue dotnet/sdk#1791

could you please link me any official doc about the property?

Conditional inclusion: https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-conditional-constructs?view=vs-2022 AdditionalProperties: https://learn.microsoft.com/en-us/visualstudio/msbuild/msbuild-task?view=vs-2022#additionalproperties-metadata

m3nax avatar Sep 22 '22 18:09 m3nax

I mean TargetFramework= in AdditionalProperties

tg123 avatar Sep 22 '22 19:09 tg123

looks like the idea is to override the TargetFramework during compile not sure if it works, but lets test with a netstandard2 only testcase to ensure the code path is covered well

thanks for the idea

tg123 avatar Sep 22 '22 22:09 tg123

Yes you are right, is the equivalent of dotnet build -p:TargetFramework=netstandard2.1 but only for a specific dependency. Tomorrow i will write the netstandard2.0 version.

m3nax avatar Sep 23 '22 05:09 m3nax

@tg123

For netstandard2.0 the project to test is only KubernetesClient.Classic.Tests correct?

m3nax avatar Sep 23 '22 08:09 m3nax

@tg123

For netstandard2.0 the project to test is only KubernetesClient.Classic.Tests correct?

netstardard 2.0 is for net48, but asp.net core cant start with net48. the tests is very basic at the moment.

Ah, I think we do not have to do this tricky workaround. the test project can target netstandard2.0/netstardard2.1 directly. If i remember correctly, the reason I am using the netcore is because of aspnetcore

while, the aspnetcore is to mock a http server here until a better http lib for test found. hope above would help you understand the whole picture better.

lets target netstanard2 directly to see if we start there :)

tg123 avatar Sep 23 '22 21:09 tg123

@tg123

I removed the tests for the netstandard2.0 version while keeping the dependency on the netstandard2.0 version of the dependency in the project file. csproj line > (https://github.com/kubernetes-client/csharp/pull/1029/files#diff-e19de45fad07027102b277638b6008d1dd89bc8cc9082513e0c86931e47a8a83R40) This allows us to test the netstandard2.0 version for the net framework 4.8

I merged the latest version of the master, are there any other changes to make?

m3nax avatar Oct 04 '22 17:10 m3nax

With this version:

  • KubernetesClient.Classic.Tests can be deleted, all netstandard2.0 tests are executed in KubernetesClient.Tests like netstandard2.1, net5.0 and net6.0
  • KubernetesClient.Classic can be deleted/deprecated, now KubernetesClient is compiled with netstandard2.0;netstandard2.1;net5.0;net6.0 targets frameworks
  • watch test (WatchTests.cs) aren't executed only for netstandard2.0 because they are unsopported (#901)
  • with netstandard2.0 version of KubernetesClient we support .NET framework version >= 4.6.2 (i think)
  • the version of KubernetesClient included by tests projects is parametrized and can be used for other tests

Steps to test this version (netstandard2.0):

# clean solution

dotnet build -f netstandard2.0 --configuration Debug -v minimal ./src/KubernetesClient/KubernetesClient.csproj 
dotnet build -f netstandard2.0 --configuration Debug -v minimal ./tests/SkipTestLogger/SkipTestLogger.csproj
		  
dotnet build --configuration Debug -v minimal --no-dependencies -p:KubernetesClientVersion="netstandard2.0" ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj 
dotnet build --configuration Debug -v minimal --no-dependencies -p:KubernetesClientVersion="netstandard2.0" ./tests/E2E.Tests/E2E.Tests.csproj
		  
dotnet test ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj /p:CollectCoverage=true /p:ExcludeByFile=\"**/KubernetesClient/generated/**/*.cs\" /p:CoverletOutputFormat="cobertura" /p:KubernetesClientVersion="netstandard2.0" --no-build 
dotnet test ./tests/E2E.Tests/E2E.Tests.csproj /p:CollectCoverage=true /p:ExcludeByFile=\"**/KubernetesClient/generated/**/*.cs\" /p:CoverletOutputFormat="cobertura" /p:KubernetesClientVersion="netstandard2.0" --no-build

Steps to test this version (netstandard2.1):

# clean solution

dotnet build -f netstandard2.1 --configuration Debug -v minimal ./src/KubernetesClient/KubernetesClient.csproj 
dotnet build -f netstandard2.0 --configuration Debug -v minimal ./tests/SkipTestLogger/SkipTestLogger.csproj
		  
dotnet build --configuration Debug -v minimal --no-dependencies -p:KubernetesClientVersion="netstandard2.1" ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj 
dotnet build --configuration Debug -v minimal --no-dependencies -p:KubernetesClientVersion="netstandard2.1" ./tests/E2E.Tests/E2E.Tests.csproj
		  
dotnet test ./tests/KubernetesClient.Tests/KubernetesClient.Tests.csproj /p:CollectCoverage=true /p:ExcludeByFile=\"**/KubernetesClient/generated/**/*.cs\" /p:CoverletOutputFormat="cobertura" /p:KubernetesClientVersion="netstandard2.1" --no-build 
dotnet test ./tests/E2E.Tests/E2E.Tests.csproj /p:CollectCoverage=true /p:ExcludeByFile=\"**/KubernetesClient/generated/**/*.cs\" /p:CoverletOutputFormat="cobertura" /p:KubernetesClientVersion="netstandard2.1" --no-build

m3nax avatar Oct 04 '22 21:10 m3nax

/assign @tg123

If you don't like the proposed solution, close the PR.

m3nax avatar Oct 10 '22 06:10 m3nax

@m3nax: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Oct 12 '22 06:10 k8s-ci-robot

Closed because implemented in https://github.com/kubernetes-client/csharp/pull/1122 with netstandard2.1 removal

m3nax avatar Dec 12 '22 11:12 m3nax