Skip to content

Conversation

agocke
Copy link
Member

@agocke agocke commented Sep 29, 2025

The primary advantage in this change is that test assemblies are now executables, not libraries. This makes debugging easier, as the executable can be run directly under the debugger. It also makes deployment of the tests easier, as we only need to copy the executable and set DOTNET_ROOT to the pre-built framework.

This is also a kind of trial-balloon. Many other tests (esp. libraries tests) in this repo would benefit from the execution advantages. However, they are built in more configurations today and are more complex. The host tests are an easier lift.

There are a few breaking changes that have been fixed by this PR:

  • xunit3 already defines a class called TestContext, so I needed to rename the existing TestContext to HostTestContext to avoid conflicts
  • Some command-line switches have changed names
  • Skip logic has changed. Arcade does not have a v3 PlatformSpecific feature. I've instead defined a PlatformSpecificFact/PlatformSpecificTheory that do basically the same job. We might want to move this class into Arcade
  • xunit3 now supports an Assert.Skip for runtime skipping. I've utilized this in a few places (especially class constructors) to replace PlatformSpecific on classes, or simplify the skip handling.

The primary advantage in this change is that test assemblies are now
executables, not libraries. This makes debugging easier, as the
executable can be run directly under the debugger. It also makes
deployment of the tests easier, as we only need to copy the executable
and set DOTNET_ROOT to the pre-built framework.

This is also a kind of trial-balloon. Many other tests (esp. libraries
tests) in this repo would benefit from the execution advantages.
However, they are built in more configurations today and are more
complex. The host tests are an easier lift.
@Copilot Copilot AI review requested due to automatic review settings September 29, 2025 22:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the hosting tests from XUnit 2 to XUnit 3 to leverage the advantage of test assemblies being executables rather than libraries, which simplifies debugging and deployment. This change also serves as a trial for potentially moving other test suites to XUnit 3.

Key changes include:

  • Updating project files to use XUnit 3 packages and set OutputType to Exe
  • Renaming TestContext to HostTestContext to avoid naming conflicts with XUnit 3's TestContext class
  • Replacing PlatformSpecific attributes with custom implementations compatible with XUnit 3
  • Using Assert.Skip for runtime skipping instead of ConditionalFact/ConditionalTheory

Reviewed Changes

Copilot reviewed 64 out of 64 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/installer/tests/TestUtils/TestUtils.csproj Updates package references to XUnit 3 extensions
src/installer/tests/TestUtils/HostTestContext.cs Renames TestContext class to HostTestContext to avoid XUnit 3 conflicts
src/installer/tests/TestUtils/PlatformSpecific*.cs Adds custom platform-specific test attributes for XUnit 3 compatibility
src/installer/tests/*/Tests.csproj Sets OutputType to Exe and TestRunnerName to XUnitV3
src/installer/tests/Directory.Build.props Updates test runner configuration for XUnit 3
Multiple test files Updates references from TestContext to HostTestContext throughout

Copy link
Contributor

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

@Youssef1313
Copy link
Member

I'm concerned about a partial move to MTP here, which is an unsupported scenario, esp if you have situations where you run via dotnet test and not via Arcade.

See https://learn.microsoft.com/dotnet/core/testing/unit-testing-with-dotnet-test for how dotnet test works. If it's still not unclear, we can take it offline to explain more, and also take it as an opportunity to improve our docs.

Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

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

Please be aware of dotnet/arcade#16003. I haven't looked at how you test different architectures, but I wanted to note about a potential break due to that issue.

@agocke
Copy link
Member Author

agocke commented Oct 4, 2025

I'm concerned about a partial move to MTP here

I don't follow. What makes this partial? In what scenarios would we use vstest?

@Youssef1313
Copy link
Member

Youssef1313 commented Oct 4, 2025

I'm concerned about a partial move to MTP here

I don't follow. What makes this partial? In what scenarios would we use vstest?

In all other projects that are xunit 2 and you invoke dotnet test, that's VSTest. Mixing MTP and VSTest in the same dotnet test invocation is not supported, is hard to get correctly in MTP v1, and is blocked in MTP v2.

@agocke
Copy link
Member Author

agocke commented Oct 4, 2025

Ah, so you're talking about tests that aren't part of this set, i.e. the hosting tests. Don't worry about it. Each of our partitions has its own constraints and runs with its own policy. Think of these tests like they live in a different repo.

Eventually, I'd like to move everything, but changing the whole runtime repo in one PR isn't a good idea.

@agocke
Copy link
Member Author

agocke commented Oct 4, 2025

I haven't looked at how you test different architectures,

We'll do it by publishing different binaries. The north star here is to simply run binaries on the target platforms.

@Youssef1313
Copy link
Member

@agocke In that case, that's fine. I just wanted to be explicit that if you have any scripts or local flows where you attempt to run multiple projects in the same dotnet test invocation, that it's not supported and won't work in MTP v2.

The existing PlatformSpecific attributes are now available in Arcade. Their usage
should still probably be retired since it makes a plain 'dotnet run' fail, but that
can be a later improvement
@agocke
Copy link
Member Author

agocke commented Oct 14, 2025

I believe this PR is up to date. I've merged in the Arcade changes, kept ConditionalTheory/Fact when possible, and kept the existing PlatformSpecific features. I think we can handle rewriting those in a separate PR.

@agocke agocke enabled auto-merge (squash) October 14, 2025 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants