Migrate test suite from xUnit v2 to xUnit v3 (#4139)#4143
Merged
Conversation
Replace xUnit v2 (xunit 2.9.3 + Xunit.SkippableFact + xunit.runner.visualstudio + xunit v2 console runner) with xUnit v3 across every test project, and drop two hand-rolled infrastructures in favour of native v3 features. Runners: - Desktop `dotnet test` projects now run on Microsoft.Testing.Platform (MTP): OutputType=Exe, TestingPlatformDotnetTestSupport + UseMicrosoftTestingPlatformRunner, xunit.v3 + Microsoft.Testing.Extensions.TrxReport/HangDump. Removed Microsoft.NET.Test.Sdk, xunit.runner.visualstudio and XunitXml.TestLogger. - Device (MAUI) and WASM (Blazor) in-app runners use DeviceRunners *.Xunit3 (AddXunit3()). - Shared test library references xunit.v3.extensibility.core + xunit.v3.assert so it stays a class library. Dynamic skip (drops Xunit.SkippableFact): - [SkippableFact]/[SkippableTheory] -> [Fact]/[Theory] - Skip.If/Skip.IfNot -> Assert.SkipWhen/Assert.SkipUnless - throw new SkipException(...) -> Assert.Skip(...) - BaseTest skip helpers updated accordingly. Assembly fixtures (drops the custom test framework): - Delete tests/Tests/Xunit/CustomTestFramework.cs, AssemblyFixtureAttribute.cs and the [assembly: TestFramework] in Properties/AssemblyInfo.cs. - [assembly: AssemblyFixture(typeof(GarbageCleanupFixture))] rebinds to v3's native Xunit.AssemblyFixtureAttribute. Other v3 source fixes: - ITestOutputHelper moved from Xunit.Abstractions to Xunit. - IAsyncLifetime.InitializeAsync/DisposeAsync now return ValueTask (SKUITests, AppiumFixture). Cake (scripts/infra/tests/test-shared.cake): - RunDotNetTest forwards MTP args after `--`: trx report + hang dump (--hangdump --hangdump-timeout 15m --hangdump-type Mini) into the results directory. This is the MTP equivalent of the VSTest --blame-hang-timeout flags from #4142; MTP HangDump has no "none" dump type so the smallest (Mini) is used. --ignore-exit-code 8 keeps all-skipped runs from failing. - RunTests (.NET Framework) executes the built v3 exe directly with the same MTP args instead of the xunit v2 console runner. - Removed the #tool xunit.runner.console reference. nuget.config: - Add nuget.org scoped via packageSourceMapping to ONLY the DeviceRunners.*.Xunit3 packages (not yet mirrored to dotnet-public); every other package still restores exclusively from the dnceng mirrors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
📦 Try the packages from this PRWarning Do not run these scripts without first reviewing the code in this PR. Step 1 — Download the packages bash / macOS / Linux: curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 4143PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4143"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4143/packages --name skiasharp-pr-4143More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4143 |
…asking Fix SkiaSharp.Tests.Integration build: - Revert two stray SKSamplingOptions.Default arguments added to DrawBitmap/ DrawImage in PlatformTestBase.cs. Those overloads don't exist in the published SkiaSharp package the integration harness targets (it consumes released NuGets, not the local build), so they broke the build. Unrelated to the xUnit migration. - Guard the MAUI skip with `if (reason != null) Assert.Skip(reason)` to drop a CS8604 nullable warning. Revert unrelated API scope creep in the shared test source: - The migration had added SKSamplingOptions.Default to 22 DrawImage/DrawBitmap/ DrawAtlas calls across 7 files. The no-sampling overloads still exist, so these were gratuitous changes that silently altered which overload each test exercises. Reverted to match main; the remaining test diff is now purely xUnit. Make MTP exit-code-8 handling opt-in instead of global: - Microsoft.Testing.Platform returns exit code 8 for BOTH "no tests discovered" and "every test dynamically skipped" (skipped tests are not counted as run, and the two are indistinguishable by exit code; --minimum-expected-tests does not separate them either). Blanket --ignore-exit-code 8 therefore also hid genuine zero-discovery misconfigurations. - RunTests/RunDotNetTest now take allowNoTests (default false). Only the hardware-gated Vulkan and Direct3D suites pass it (an agent without the GPU backend legitimately skips everything, matching xUnit v2 all-skipped-is-success). The main and SingletonInit suites stay strict: a zero-test run fails the leg. Remove the now-unused is32 parameter from RunTests: - With xUnit v3 each architecture builds its own runnable exe (the net48 x86 build produces a genuinely 32-bit exe), so the old console-runner bitness selector is gone. 32-bit and .NET Framework legs are unchanged: tests-netfx.cake still builds and runs net48 for both x86 and x64. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
📖 Documentation Preview The documentation for this PR has been deployed and is available at: 🔗 View Staging Site This preview will be updated automatically when you push new commits to this PR. This comment is automatically updated by the documentation staging workflow. |
- Add a pure-managed always-run smoke test to the Vulkan and Direct3D
suites. Each exercises a backend-specific SkiaSharp type that needs no
GPU runtime: GRVkImageInfo (a Vulkan interop struct) and
GRD3DTextureResourceInfo (a Direct3D resource descriptor). These suites
are otherwise entirely hardware-gated and skip every test on agents
without the backend; under Microsoft.Testing.Platform a run that
executes zero tests returns exit code 8 (failure). The smoke test
guarantees >=1 executed test so the suite passes legitimately.
- Remove the `allowNoTests` / `--ignore-exit-code 8` masking entirely
(test-shared.cake, tests-netfx.cake, tests-netcore.cake). With a
guaranteed always-run test in every suite, no leg can legitimately
all-skip, so every leg is now strict — no exit codes are suppressed.
Verified: Vulkan suite runs 7 tests (2 run incl. the GRVkImageInfo
smoke test, 5 skipped) and exits 0.
- Fix CI test-results publishing for the desktop legs. MTP's TrxReport
emits TestResults.trx (TRX), but the four desktop PublishTestResults
steps still expected xUnit/TestResults.xml, so results (including the
32-bit net48 run) would have stopped publishing after the MTP switch.
Switch them to VSTest/*.trx, matching the device legs.
- 32-bit is exercised in CI: the tests-netfx leg loops {x86, x64} and CI
passes no --arch filter, so both the 32-bit and 64-bit net48 runs
execute (and now publish results).
- Integration harness: default SkiaSharp/HarfBuzzSharp to a floating "*"
version so a bare run tests the latest stable on the configured feeds
(resolves to SkiaSharp 3.119.4 / HarfBuzzSharp 8.3.1.5 today) instead
of an arbitrary pinned 3.119.1. The release pipeline still overrides
with the exact version being verified.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mattleibow
force-pushed
the
mattleibow/dev-issue-4139-xunit-v3
branch
from
fac71c6 to
316e320
Compare
Follow-up from dual-model PR review: - Add the same always-run managed-only smoke test to the migrated SkiaSharp.Views.Gtk4.Tests project. Its other tests initialise native GTK4 in their constructors and skip every test on a headless/GTK-less agent; under Microsoft.Testing.Platform that all-skipped run would exit 8 (failure). The smoke test exercises pure-managed SkiaSharp geometry types (SKPointI/SKSizeI — no GTK, no native call). Verified: the suite runs 30 tests (1 executed, 29 skipped) and exits 0. (This project is not currently wired into CI, but it was migrated to MTP, so the guard keeps it safe if it is ever run standalone or added to a leg.) - Document in test-shared.cake why the hang-dump uses `--hangdump-type Mini` rather than #4142's VSTest `none`: MTP only detects a per-test hang via the HangDump extension, which always writes a dump (no "none" type exists); the global `--timeout` aborts without a dump but is a whole-session timeout, not per-test, so it is unsuitable. Mini is the smallest dump and only materialises on an actual hang. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The DeviceRunners.VisualRunners.Xunit3 and DeviceRunners.UITesting.Xunit3 packages (0.1.0-preview.11) are now available on the dnceng dotnet-public mirror, so the temporary nuget.org package source and its source-mapping scope are no longer needed. All packages now restore exclusively from the dnceng mirrors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add SkiaSharp.Views.Gtk4.Tests to the .NET Core test list so it builds and runs on every desktop leg (Windows/macOS/Linux). The suite skips its native GTK4 tests when the libraries are unavailable and always runs a managed smoke test, so it is safe on agents without GTK4. - Install libgtk-4-1 on the Linux .NET Core test leg so the GTK4 conversion tests actually execute there rather than all-skipping. - Pin SkiaSharp.Tests.Integration default versions to explicit latest stable (SkiaSharp 3.119.4 / HarfBuzzSharp 8.3.1.5) instead of a floating "*". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert the Integration project to main's design: SkiaSharpVersion / HarfBuzzSharpVersion have no default and the ValidateVersions target errors if they are not supplied. This avoids both a floating "*" version and a hardcoded version that goes stale; the caller (release pipeline / release-testing) always passes the exact version under test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… libgtk arg - nuget.config: now that the DeviceRunners.*.Xunit3 packages are mirrored to dotnet-public, remove the temporary nuget.org source and its package source mapping entirely, restoring nuget.config to match main exactly (no divergence, no stale comment). - Reindent VulkanTests/SmokeTest.cs and Direct3D SmokeTest.cs with tabs to match repo style. - Remove the leftover 'libgtk-4-1' from the netcore Linux test leg (it now lives in MANAGED_LINUX_PACKAGES). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gtk_init() calls exit() when no display is available, which killed the entire Microsoft.Testing.Platform test host (exit code 7) on the headless Linux agent where libgtk-4 is installed. The managed try/catch cannot intercept a native exit(). Switch SKDrawingAreaTest.InitGtk() to gtk_init_check() (Gtk.Functions.InitCheck), which returns false instead of aborting, so the display-dependent drawing-area tests skip gracefully while the GTK conversion tests still execute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gtk_init_check() also aborts the process (native exit, 'cannot open display') on the headless Linux agent, not just gtk_init() - so switching to InitCheck() was not enough and the test host still crashed (exit 7). Skip the display-dependent SKDrawingArea tests up-front using a pure-managed DISPLAY/WAYLAND_DISPLAY environment check, before calling into any GTK display API. The GTK conversion tests (which need only the native libraries, not a display) continue to run and pass on Linux. Validated locally on macOS with GTK4 brew libs loadable and no display: 27 passed, 3 skipped, exit 0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…4139-xunit-v3 # Conflicts: # scripts/azure-templates-variables.yml # tests/Tests/SkiaSharp/SKBitmapTest.cs
mattleibow
added a commit
that referenced
this pull request
Jun 25, 2026
…release-testing skill filter syntax (#4244) [Tests] Fix integration harness Docker detection + command timeouts, and release-testing filter syntax (#4244) Context: found during 4.150.0-preview.2 release testing Context: Microsoft.Testing.Platform migration #4143 Three latent reliability bugs in the SkiaSharp.Tests.Integration harness (and the release-testing skill) caused the Linux/Docker test to be silently skipped, made command timeouts non-functional, and left the release skill running every test instead of the one requested. ~~ LinuxConsoleTests.IsDockerAvailable() deadlock ~~ The probe set RedirectStandardOutput/Error = true but never drained the pipes before WaitForExit(10000). `docker info` can emit well over the 64KB OS pipe buffer, so the child blocked on write while the parent's 10s wait expired; the subsequent ExitCode read then reported Docker as unavailable even when the daemon was running. Net effect: the Linux container tests were ALWAYS skipped. Drain stdout/stderr via ReadToEndAsync() before WaitForExit, kill the child on timeout, then Task.WaitAll the reads so a large payload can no longer deadlock the probe. Verified live: the Linux tests went from Skipped to actually executing once a real daemon was present. ~~ PlatformTestBase.Run() timeout never enforced ~~ Run() awaited StandardOutput.ReadToEndAsync()/StandardError.ReadToEndAsync() BEFORE WaitForExit(timeoutSeconds * 1000). ReadToEndAsync only completes when the stream closes (process exit), so the await blocked until the process finished and the timeout was effectively dead code — a hung `docker build` ran unbounded. Reading stdout fully before starting stderr could itself deadlock if stderr filled. Start both pipe reads first, enforce the timeout via WaitForExit while the reads are in flight (Kill(true) on expiry), then await the drained output. Verified live: SkiaSharpRunsOnLinux passed end-to-end driving a long `docker build`/`run`, and a cold-cache build correctly threw TimeoutException after 180s — the exact behavior that was previously impossible to reach. ~~ release-testing skill uses stale VSTest --filter syntax ~~ After the xUnit v2 -> v3 / Microsoft.Testing.Platform migration (#4143), VSTest-style `dotnet test --filter "FullyQualifiedName~X"` is silently ignored under MTP and runs ALL tests. The skill still documented that form. Update every command to the MTP form — filter args after the `--` separator using --filter-class / --filter-method / --filter-namespace with `*` wildcards — keeping MSBuild -p: properties (SkiaSharpVersion, iOSDevice, AndroidDeviceId, etc.) BEFORE the separator. Added a note explaining the MTP requirement so the legacy syntax is not reintroduced. Verified against the latest published packages (SkiaSharp 4.150.0-preview.2.1 / HarfBuzzSharp 14.2.1-preview.2.1): SmokeTests, ConsoleTests, and both LinuxConsoleTests pass, and --filter-class/--filter-method correctly narrow the run under MTP. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A complete switch of the SkiaSharp test suite from xUnit v2 (
xunit2.9.3 +Xunit.SkippableFact+xunit.runner.visualstudio+ the xUnit v2 console runner) to xUnit v3, across every test project. Driven by #4139 (macOS CI hangs). Two hand-rolled infrastructures are dropped in favour of native v3 features.Separate from Task 1 PR #4142 (blame-hang-timeout) — see reconciliation note below.
Runners
dotnet testprojects → Microsoft.Testing.Platform (MTP):OutputType=Exe,TestingPlatformDotnetTestSupport+UseMicrosoftTestingPlatformRunner,xunit.v3+Microsoft.Testing.Extensions.TrxReport/HangDump. RemovedMicrosoft.NET.Test.Sdk,xunit.runner.visualstudio,XunitXml.TestLogger.*.Xunit3(.AddXunit3()), pinned at0.1.0-preview.11.xunit.v3.extensibility.core+xunit.v3.assertso it stays a class library.SkiaSharp.Views.Gtk4.Tests) migrated to v3/MTP and wired into thetests-netcoreleg so its conversion tests run in CI (previously not executed at all)..NET Framework + 32-bit are unchanged:
tests-netfx.cakestill builds and runs net48 for both x86 and x64. With v3, each architecture builds its own runnable exe (aPlatform=x86net48 build is a genuinely 32-bit exe), so the old console-runner bitness selector (is32) was removed as dead code.Native dynamic skip (drops
Xunit.SkippableFact)[SkippableFact]/[SkippableTheory]→[Fact]/[Theory]Skip.If/Skip.IfNot→Assert.SkipWhen/Assert.SkipUnlessthrow new SkipException(...)→Assert.Skip(...)Assembly fixtures (drops the custom test framework)
CustomTestFramework.cs,AssemblyFixtureAttribute.cs, and the[assembly: TestFramework].[assembly: AssemblyFixture(typeof(GarbageCleanupFixture))]rebinds to v3's nativeXunit.AssemblyFixtureAttribute.Other v3 source fixes
ITestOutputHelpermoved fromXunit.AbstractionstoXunit.IAsyncLifetime.InitializeAsync/DisposeAsyncnow returnValueTask(SKUITests,AppiumFixture).Zero-executed-test guard (no masking)
MTP returns exit code 8 when zero tests execute — and it counts a dynamically-skipped test as not run, so a suite where every test skips also exits 8 (xUnit v2/VSTest treated all-skipped as success). The fully hardware-gated Vulkan and Direct3D suites skip every test on agents without that GPU backend.
Rather than suppress exit 8 (which would also hide a genuine zero-discovery misconfiguration), each of those suites gets a small always-run
SmokeTestthat exercises a backend-specific SkiaSharp type needing no GPU runtime —GRVkImageInfo(a Vulkan interop struct) andGRD3DTextureResourceInfo(a Direct3D resource descriptor). That guarantees ≥1 executed test, so the suite passes legitimately while every other test still skips. With that guard in place there is no--ignore-exit-code/allowNoTestsmasking anywhere — every leg is strict, and a real zero-test run still fails.The same problem applies to the headless Linux Gtk4 leg:
gtk_init/gtk_init_checkcall nativeexit()when there is no display (uncatchable from managed code, kills the MTP host). The three display-dependent drawing tests are gated behind a pure-managedDISPLAY/WAYLAND_DISPLAYcheck before any GTK call; the ~26 conversion tests (which need only the native libs, not a display) still execute, plus an always-run managed smoke test. Thelibgtk-4-1package is added to the Linux agent so those conversion tests run for real instead of skipping.Hang protection / #4142 reconciliation
RunDotNetTestforwards MTP hang-dump args (--hangdump --hangdump-timeout 15m --hangdump-type Mini) into the results directory — the MTP equivalent of #4142's VSTest--blame-hang-timeout. MTP HangDump has nononedump type, so the smallest (Mini) is used. These supersede #4142's VSTest blame flags on merge.Package sourcing
DeviceRunners.*.Xunit3(0.1.0-preview.11) are now mirrored to the dncengdotnet-publicfeed, sonuget.configrestores exclusively from the two dnceng mirrors (dotnet-public+dotnet-eng) — nonuget.orgsource and nopackageSourceMappingscoping is required. No package uses a floating*version.Validation
SkiaSharp.Tests.Integrationbuilds (release harness, against an explicitly-supplied package version).dotnet testMTP pipeline validated end-to-end: host launch, discovery + execution, native dynamic skip reported correctly, trx written, hangdump active.TestResults.trx, so the four desktop publish steps were switched fromxUnit/TestResults.xmltoVSTest/*.trx(matching the device legs). Without this, desktop results — including the 32-bit run — would silently stop publishing.SkiaSharp.Tests.Integrationrequires an explicit package version — there is no default and no floating*; a bare run with no-p:SkiaSharpVersion=fails fast via aValidateVersionstarget. The release pipeline passes the exact version under test.Not verified locally (needs CI/hardware)
tests-netfxleg loops{x86, x64}and CI passes no--arch, so both run and publish.mainsurfaces at test runtime only — out of scope for this migration.Closes #4139.