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.

Sign up for GitHub

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

Jump to bottom

Build tests on Linux and Windows #47106

Merged
merged 7 commits into from
Feb 27, 2025
Merged

Build tests on Linux and Windows #47106

merged 7 commits into from
Feb 27, 2025

Conversation

NikolaMilosavljevic
Copy link
Member

Fixes: dotnet/source-build#4909

Instead of building tests on Windows x64 in PR we will now have:

  • Building tests on Linux x64 in PR
  • Building tests on Windows x64 in CI

This change brings back the PR validation time to less than 2 hours.

@Copilot Copilot bot review requested due to automatic review settings February 25, 2025 18:45
@NikolaMilosavljevic NikolaMilosavljevic requested review from a team as code owners February 25, 2025 18:45
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Infrastructure untriaged Request triage from a team member labels Feb 25, 2025
Copilot
Copilot AI reviewed Feb 25, 2025
View reviewed changes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

eng/pipelines/templates/stages/vmr-build.yml:857

  • The Windows_BuildTests job is now relocated into the vertical build section. Please verify that its new placement and order in the pipeline is intentional.
- template: ../jobs/vmr-build.yml

eng/pipelines/templates/stages/vmr-build.yml:862

  • The addition of the 'sign: false' parameter could modify the expected build signing behavior for Windows builds. Ensure that this change aligns with the intended CI process.
sign: false
@MichaelSimons
Copy link
Member

Just to clarify, we are already building tests in a linux x64 leg. So this PR is just about moving the Windows test leg to just be run in CI.

@NikolaMilosavljevic
Copy link
Member Author

Just to clarify, we are already building tests in a linux x64 leg. So this PR is just about moving the Windows test leg to just be run in CI.

We are not - existing Linux job now gets the extraProperties: /p:DotNetBuildTests=true parameter from deleted/moved job.

ViktorHofer
ViktorHofer previously requested changes Feb 25, 2025
View reviewed changes
Copy link
Member

@ViktorHofer ViktorHofer left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want that leg to run in official builds. It would also be sweet if we could keep the leg in the "VMR Vertical Build Validation" stage. Long term we want a separate pipeline for it but for now I think it makes sense to condition it to run in public rolling builds only.

@NikolaMilosavljevic
Copy link
Member Author

We don't want that leg to run in official builds. It would also be sweet if we could keep the leg in the "VMR Vertical Build Validation" stage. Long term we want a separate pipeline for it but for now I think it makes sense to condition it to run in public rolling builds only.

Fixed with e45016a

This was referenced Feb 26, 2025
Open
Open
Open
ViktorHofer
ViktorHofer reviewed Feb 26, 2025
View reviewed changes
eng/pipelines/templates/stages/vmr-build.yml
vmrBranch: ${{ variables.VmrBranch }}
pool: ${{ parameters.pool_Windows }}
targetOS: windows
targetArchitecture: x64
extraProperties: /p:DotNetBuildTests=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider updating the job name to make it clear that this job now builds tests

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 936d376

@ViktorHofer ViktorHofer self-requested a review February 26, 2025 07:57
@NikolaMilosavljevic
Copy link
Member Author

I've merged the latest from target branch, hopefully that resolves the issue that showed when building tests on Ubuntu - I did not see it in my local build.

##[error]src/sdk/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj(0,0): error NU1603: (NETCORE_ENGINEERING_TELEMETRY=Restore) 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.

@NikolaMilosavljevic
Copy link
Member Author

I've merged the latest from target branch, hopefully that resolves the issue that showed when building tests on Ubuntu - I did not see it in my local build.

##[error]src/sdk/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj(0,0): error NU1603: (NETCORE_ENGINEERING_TELEMETRY=Restore) 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.

My local build didn't include the change that enabled SDK tests on non-Windows - #47103

@ViktorHofer @mmitche are we still missing some changes for building Microsoft.TestPlatform.TestHost package on Linux?

@NikolaMilosavljevic NikolaMilosavljevic mentioned this pull request Feb 26, 2025
Merged
@NikolaMilosavljevic
Copy link
Member Author

I've merged the latest from target branch, hopefully that resolves the issue that showed when building tests on Ubuntu - I did not see it in my local build.
##[error]src/sdk/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Microsoft.NET.Build.Tasks.UnitTests.csproj(0,0): error NU1603: (NETCORE_ENGINEERING_TELEMETRY=Restore) 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.

My local build didn't include the change that enabled SDK tests on non-Windows - #47103

@ViktorHofer @mmitche are we still missing some changes for building Microsoft.TestPlatform.TestHost package on Linux?

I've pushed a fix for this - SDK tests are not building on Linux due to an underlying issue with TestHost. I have also reopened the tracking issue.

@NikolaMilosavljevic NikolaMilosavljevic merged commit 88f23c0 into dotnet:main Feb 27, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

@ViktorHofer ViktorHofer ViktorHofer approved these changes

@MichaelSimons MichaelSimons MichaelSimons approved these changes

Assignees
No one assigned
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build tests in Linux x64 leg
3 participants
@NikolaMilosavljevic @MichaelSimons @ViktorHofer