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

Add VMR leg that builds test projects #44843

Merged
merged 21 commits into from
Feb 6, 2025
Merged

Conversation

ViktorHofer
Copy link
Member

Fixes dotnet/source-build#4168

Running as part of the PR to get some initial results.

Fixes dotnet/source-build#4168

Running as part of the PR to get some initial results.
@ViktorHofer ViktorHofer requested review from a team as code owners November 13, 2024 13:08
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Nov 13, 2024
@ViktorHofer
Copy link
Member Author

ViktorHofer commented Nov 13, 2024

src\winforms\src\System.Windows.Forms.Design\tests\UnitTests\System\Windows\Forms\Design\ToolStripContentPanelDesignerTests.cs(43,67): error CS8620: (NETCORE_ENGINEERING_TELEMETRY=Build) Argument of type 'string[]' cannot be used for parameter 'span' of type 'Span<string?>' in 'bool MemoryExtensions.Contains<string?>(Span<string?> span, string? value)' due to differences in the nullability of reference types.

@am11 did we see this one when we updated the VMR to the .NET 10 SDK? Can't remember if we talked about it.

@am11
Copy link
Member

am11 commented Nov 13, 2024

Yup, it was #43015 (comment). It will probably need a fix like dotnet/winforms@main...am11:winforms:patch-1. I'll try building it on windows soon.

@ViktorHofer
Copy link
Member Author

    D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\CustomConvertersTestBase.cs(1187,49): error CS0023: Operator '.' cannot be applied to operand of type 'void' [D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj]
    D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\CustomConvertersTestBase.cs(1188,49): error CS0023: Operator '.' cannot be applied to operand of type 'void' [D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj]
    D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\Query\PrimitiveCollectionsQueryTestBase.cs(430,67): error CS8620: Argument of type 'string[]' cannot be used for parameter 'span' of type 'Span<string?>' in 'bool MemoryExtensions.Contains<string?>(Span<string?> span, string? value)' due to differences in the nullability of reference types. [D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj]
    D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\Query\PrimitiveCollectionsQueryTestBase.cs(433,68): error CS8620: Argument of type 'string[]' cannot be used for parameter 'span' of type 'Span<string?>' in 'bool MemoryExtensions.Contains<string?>(Span<string?> span, string? value)' due to differences in the nullability of reference types. [D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj]
    D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\Query\PrimitiveCollectionsQueryTestBase.cs(577,67): error CS8620: Argument of type 'string[]' cannot be used for parameter 'span' of type 'Span<string?>' in 'bool MemoryExtensions.Contains<string?>(Span<string?> span, string? value)' due to differences in the nullability of reference types. [D:\a\_work\1\vmr\src\efcore\test\EFCore.Specification.Tests\EFCore.Specification.Tests.csproj]

@am11 would you want to help me with driving this one in? I currently have some other obligations and won't be able to get back to this PR anytime soon. See the above efcore test failure. They need the same first class Span reaction update.

There's also a winforms test failure in the PR that I don't really understand yet.

@am11
Copy link
Member

am11 commented Nov 19, 2024

I think #44922 is bringing winforms change. I'll check efcore.

@ViktorHofer
Copy link
Member Author

The efcore issue is weird:

2024-11-20T20:05:12.6651643Z     FSC : error FS2014: A problem occurred writing the binary 'D:\a\_work\1\vmr\src\efcore\artifacts\obj\EFCore.FSharp.FunctionalTests\Release\net9.0\refint\Microsoft.EntityFrameworkCore.FSharp.FunctionalTests.dll': A call to StrongNameSignatureSize failed (Invalid Public Key blob) [D:\a\_work\1\vmr\src\efcore\test\EFCore.FSharp.FunctionalTests\EFCore.FSharp.FunctionalTests.fsproj]

The winforms failure is also still happening.

@am11
Copy link
Member

am11 commented Nov 20, 2024

Looks like a known bug dotnet/fsharp#17451. A solution (workaround?) is here dotnet/fsharp#11546 (comment).

Since it is a test assembly perhaps we should just disable strong name signing?

@ViktorHofer
Copy link
Member Author

It would be interesting to understand why the fsharp test build behaves differently with the VMR.

@NikolaMilosavljevic
Copy link
Member

efcore issue is caused by /p:FullAssemblySigningSupported=false which is set in VMR build. This causes PublicSign to not be set to false here: https://github.com/dotnet/arcade/blob/2fd9b394c6ef3ad22639d83b8da649c9350f88f6/src/Microsoft.DotNet.Arcade.Sdk/tools/StrongName.targets#L50

I'll try to set PublicSign to false explicitly in this test project.

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Dec 4, 2024

cc @mmitche as you approved the Arcade PR: dotnet/arcade#12940 and might have context here.

@mmitche
Copy link
Member

mmitche commented Dec 4, 2024

I believe this should be conditioned on source-only or non-source only builds, rather than a blanket setting across all VMR builds. That said, I'm slightly uncomfortable with where the setting is put. I think it should actually be in StrongName.targets or generally within the arcade infra, rather than the source build control setup.

@mmitche
Copy link
Member

mmitche commented Dec 4, 2024

I'll file an issue and open a PR to fix this.

@mmitche
Copy link
Member

mmitche commented Dec 4, 2024

dotnet/arcade#15301

@NikolaMilosavljevic
Copy link
Member

Tests build is failing in winforms with:

The command "
      call "D:\a\_work\1\vmr\src\winforms\eng\init-vs-env.cmd" 
      
       cmake -G "Visual Studio 17 2022" -A Win32 -B "D:\a\_work\1\vmr\src\winforms\artifacts\obj\NativeTests\x86\Release" -S "D:\a\_work\1\vmr\src\winforms\src\System.Windows.Forms\tests\InteropTests\NativeTests" -DCMAKE_INSTALL_PREFIX=D:\a\_work\1\vmr\src\winforms\artifacts\bin\NativeTests\x86\Release --log-level=WARNING
      " exited with code 1.

I have not seen this locally and it is likely due to host configuration issues.

I'm going to opt-out of building tests in winforms and create a tracking issue in dotnet/source-build

@NikolaMilosavljevic
Copy link
Member

Hmm, this PR is now hitting an odd error that I don't think is related.

It's failure to restore vstest package during SDK repo build:
D:\a\_work\1\vmr\src\sdk\test\Microsoft.NET.Sdk.BlazorWebAssembly.Tests\Microsoft.NET.Sdk.BlazorWebAssembly.Tests.csproj error NU1603: Warning As Error: Microsoft.NET.Test.Sdk 17.14.0-ci depends on Microsoft.TestPlatform.TestHost (>= 17.14.0-ci) but Microsoft.TestPlatform.TestHost 17.14.0-ci was not found. Microsoft.TestPlatform.TestHost 17.14.0-preview-25071-04 was resolved instead. [D:\a\_work\1\vmr\src\sdk\sdk.sln]

I have not seen this in local build, but I have not refreshed the local sha in a week. I'm updating local sha to repro this.

There were some recent changes in versioning i.e. the use of official or dev labels. @ViktorHofer @mmitche does this sound familiar?

I'm not sure which repo builds Microsoft.TestPlatform.TestHost - I don't see this produced in local build and darc tool is not working for me atm. Perhaps a new dependency that should be added in vstest or sdk.

@mmitche
Copy link
Member

mmitche commented Feb 3, 2025

Hmm, this PR is now hitting an odd error that I don't think is related.

It's failure to restore vstest package during SDK repo build: D:\a\_work\1\vmr\src\sdk\test\Microsoft.NET.Sdk.BlazorWebAssembly.Tests\Microsoft.NET.Sdk.BlazorWebAssembly.Tests.csproj error NU1603: Warning As Error: Microsoft.NET.Test.Sdk 17.14.0-ci depends on Microsoft.TestPlatform.TestHost (>= 17.14.0-ci) but Microsoft.TestPlatform.TestHost 17.14.0-ci was not found. Microsoft.TestPlatform.TestHost 17.14.0-preview-25071-04 was resolved instead. [D:\a\_work\1\vmr\src\sdk\sdk.sln]

I have not seen this in local build, but I have not refreshed the local sha in a week. I'm updating local sha to repro this.

There were some recent changes in versioning i.e. the use of official or dev labels. @ViktorHofer @mmitche does this sound familiar?

I'm not sure which repo builds Microsoft.TestPlatform.TestHost - I don't see this produced in local build and darc tool is not working for me atm. Perhaps a new dependency that should be added in vstest or sdk.

That looks like a new package dependency that is not being built. It's built out of vstest normally: https://github.com/microsoft/vstest/blob/main/src/package/Microsoft.TestPlatform.TestHost/Microsoft.TestPlatform.TestHost.csproj#L7-L8

@mmitche
Copy link
Member

mmitche commented Feb 3, 2025

@NikolaMilosavljevic #46480

@mmitche
Copy link
Member

mmitche commented Feb 3, 2025

@NikolaMilosavljevic #46480

@NikolaMilosavljevic vstest just doesn't build on non-Windows. I'm updating the conditional to pack on Windows too, but we should disable building the test on non-Windows.

@NikolaMilosavljevic
Copy link
Member

@NikolaMilosavljevic #46480

@NikolaMilosavljevic vstest just doesn't build on non-Windows. I'm updating the conditional to pack on Windows too, but we should disable building the test on non-Windows.

vstest repo gets built as part of source-build and Linux unified build. I presume we should not build vstest test projects on non-Windows, right?

@mmitche
Copy link
Member

mmitche commented Feb 3, 2025

@NikolaMilosavljevic #46480

@NikolaMilosavljevic vstest just doesn't build on non-Windows. I'm updating the conditional to pack on Windows too, but we should disable building the test on non-Windows.

vstest repo gets built as part of source-build and Linux unified build. I presume we should not build vstest test projects on non-Windows, right?

This looks different. it looks like an SDK test project, that is referencing a vstest package that is not produced on Linux.

@NikolaMilosavljevic
Copy link
Member

NikolaMilosavljevic commented Feb 3, 2025

@NikolaMilosavljevic #46480

@NikolaMilosavljevic vstest just doesn't build on non-Windows. I'm updating the conditional to pack on Windows too, but we should disable building the test on non-Windows.

vstest repo gets built as part of source-build and Linux unified build. I presume we should not build vstest test projects on non-Windows, right?

This looks different. it looks like an SDK test project, that is referencing a vstest package that is not produced on Linux.

I see - there are 60+ sdk tests that need this vstest package. Perhaps it's best to not allow sdk repo to build tests on non-Windows.

I'll create a tracking issue in source-build.

@NikolaMilosavljevic
Copy link
Member

I've merged the latest from main to pick up #46480 - this should resolve the last build issue.

@NikolaMilosavljevic
Copy link
Member

First successful build of the '_BuildTests' leg - it took 106 minutes. @ViktorHofer

We are skipping tests in several repos, i.e. aspnetcore, so that will add some time, but hopefully keep it below 2 hours limit.

There is a failure in repo's test leg, which is unrelated.

@ViktorHofer ViktorHofer marked this pull request as ready for review February 5, 2025 21:11
@ViktorHofer
Copy link
Member Author

@NikolaMilosavljevic changes LGTM

@NikolaMilosavljevic
Copy link
Member

macOS test legs are timing out. This is unrelated as the PR is not affecting the repo build.

@mmitche @MichaelSimons @marcpopMSFT can someone merge this? I don't have permissions.

@MichaelSimons MichaelSimons merged commit b513b97 into main Feb 6, 2025
34 of 38 checks passed
@MichaelSimons MichaelSimons deleted the VMRDotNetBuildTestsLeg branch February 6, 2025 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rolling CI builds should build the test projects, to validate that things still build on the merged content
8 participants