[DRAFT] Xunit2 -> Xunit3#5507
Open
mikaelweave wants to merge 28 commits intomainfrom
Open
Conversation
- Add xunit.v3, xunit.v3.mtp-v2, xunit.v3.assert, xunit.v3.common, xunit.v3.extensibility.core (3.2.2) - Add Microsoft.Testing.Extensions.Telemetry, Microsoft.Testing.Platform.MSBuild (2.0.2) - Bump Microsoft.Testing.Extensions.CodeCoverage 18.1.0 -> 18.3.1 - Bump xunit.runner.visualstudio 3.0.2 -> 3.1.5 - Remove YTest.MTP.XUnit2, xunit, xunit.assert, xunit.extensibility.core, Xunit.SkippableFact Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Enable UseMicrosoftTestingPlatformRunner - Switch xunit.runner.json from Content to None + CopyToPublishDirectory=Never - Replace YTest.MTP.XUnit2 reference with MTP extensions (Telemetry, MSBuild) - Drop xunit.runner.visualstudio remove directive (now used as normal ref) - Suppress xUnit1031, xUnit1051, SA1208, SA1209, SA1518 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- CustomAnalysisRules.Test.ruleset: suppress xUnit1031, xUnit1051 and StyleCop SA1208/SA1209 - THIRDPARTYNOTICES.md: remove YTest.MTP.XUnit2 and Xunit.SkippableFact entries - xunit.runner.json: update schema URL to v3 configuration schema Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace xunit package with xunit.v3.mtp-v2, xunit.v3.common, and xunit.v3.extensibility.core. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- CustomXunitTestFramework now inherits the v3 XunitTestFramework (configFileName ctor), overrides CreateDiscoverer/CreateExecutor taking Assembly. - CustomXunitTestFrameworkDiscoverer uses v3 XunitTestAssembly + IXunitTestClass + ValueTask-based FindTestsForType/FindTestsForMethod with Func<ITestCase, ValueTask<bool>> callback; integrates new FixtureArgumentSet* types. - CustomXunitTestFrameworkExecutor is restructured around RunTestCases + custom XunitTestAssemblyRunner/CollectionRunner/ClassRunner/TestRunner types that propagate FixtureArgumentSet variants, and uses reflection to seed the v3 XunitTestAssembly AssemblyFixtureTypes lazy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xUnit v3 needs its own IXunitTestClass/IXunitTestCollection/IXunitTestMethod implementations that carry the fixture-argument-set variants. - Add FixtureArgumentSetTestClass, FixtureArgumentSetTestCollection, FixtureArgumentSetTestMethod. - Add FixtureArgumentSetsAttribute (carries the Flags enum combinations + CollectionBehavior). - Remove obsolete TestClassWithFixtureArguments and TestClassWithFixtureArgumentsTypeInfo (v2 ITypeInfo-based). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- AssemblyFixtureAttribute, RetryFactAttribute, RetryFactDiscoverer, RetryTestCase, RetryTheoryAttribute, RetryTheoryDiscoverer updated for xUnit v3 API (Assembly-based ctors, ValueTask discovery, IXunitTestCase/IXunitTestCollection abstractions). - Add local Skip, SkippableFactAttribute, SkippableTheoryAttribute classes replacing the now-removed Xunit.SkippableFact package. Skip.If/IfNot throw SkipException.ForSkip(reason) which xUnit v3 recognizes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Swap 'xunit' PackageReference to 'xunit.v3.mtp-v2' across all 19 *UnitTests*.csproj and Microsoft.Health.Fhir.Tests.Common.csproj. - Remove 'Xunit.SkippableFact' PackageReference from Azure.UnitTests and the four *.Core.UnitTests projects (replaced by local Skip/SkippableFact types in Microsoft.Health.Extensions.Xunit). - Add ProjectReference to Microsoft.Health.Extensions.Xunit in Azure.UnitTests (needed for the local SkippableFact attribute). - Swap 'xunit.assert'/'xunit.extensibility.core' to their 'xunit.v3.*' equivalents in SchemaManager.UnitTests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Swap 'xunit' PackageReference to 'xunit.v3.mtp-v2' and drop 'Xunit.SkippableFact' from the four *.Tests.Integration.csproj files (R4, R4B, R5, Stu3). The xunit.runner.json copy metadata is handled in the shared .projitems (coming in a later commit). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Swap 'xunit' PackageReference to 'xunit.v3.mtp-v2' and drop 'Xunit.SkippableFact' and 'System.IO.FileSystem.AccessControl' across all four *.Tests.E2E.csproj files (R4, R4B, R5, Stu3). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- TestFramework now uses typeof() syntax (v3 no longer takes typeName/assemblyName string pair). - AssemblyFixture moves to Xunit namespace (fully qualified to disambiguate from Microsoft.Health.Extensions.Xunit). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xUnit v3 changes IAsyncLifetime.InitializeAsync/DisposeAsync from Task to ValueTask. Update all test fixtures and test classes that implement IAsyncLifetime, OnInitializedAsync, or OnDisposedAsync hooks accordingly. Also remove stale 'using Xunit.Abstractions;' where ITestOutputHelper moved to the Xunit namespace. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ojitems for MTP xUnit v3 MTP runner requires xunit.runner.json to be copied to the test output directory. Add the Shared.Tests.Integration xunit.runner.json and register it (plus the E2E one) in the shared .projitems with Link + CopyToPublishDirectory=Never so each downstream test project picks it up. Add v3 $schema URL. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xUnit v3 parallelizes more aggressively by default. CustomQueriesUnitTests mutates shared static state (CustomQueriesHash) so add a CollectionDefinition with DisableParallelization=true and attach the test class to it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Tests ITestOutputHelper moved to the Xunit namespace in xUnit v3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When using Microsoft Testing Platform (xUnit v3) the DotNetCoreCLI@2 'test' command no longer emits TRX automatically. Pass --report-trx, disable built-in test-result publishing, and add an explicit PublishTestResults@2 step. Keep failTaskOnFailedTests=true for the non-coverage path and keep it false for the coverage path until testfx#7167 is fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The MTP runner does not accept the legacy VSTest '--filter FullyQualifiedName~X' expressions. Switch to '--filter-query' with trait expressions (DataStore + Category) for E2E and export jobs, and use '--filter-not-class'/'--filter-not-trait' for integration-test exclusions. Also pass 'publishTestResults: false' on the integration-test DotNetCoreCLI@2 tasks so the separate PublishTestResults@2 step owns TRX publishing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MTP requires xunit.runner.json to be copied into each test project's output directory. Add a PowerShell task that asserts the file is present for every integration-test project prior to test execution, and prints its contents to the build log for diagnosis. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove 'using Xunit.Abstractions;' (namespace removed in v3; ITestOutputHelper lives in Xunit) - Add 'using System.Threading.Tasks;' where IAsyncLifetime now returns ValueTask - Add 'using Xunit;' to files that lost the only Xunit reference via the Abstractions removal Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+170
to
+186
| foreach (var fixtureType in classFixtureTypes) | ||
| { | ||
| var constructor = fixtureType.GetConstructors() | ||
| .SingleOrDefault(ctor => !ctor.IsStatic && ctor.IsPublic); | ||
| if (constructor == null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| foreach (var parameter in constructor.GetParameters()) | ||
| { | ||
| if (parameter.ParameterType.IsEnum) | ||
| { | ||
| fixtureParameterTypes.Add(parameter.ParameterType); | ||
| } | ||
| } | ||
| } |
The CustomXunitTestFrameworkDiscoverer was creating FixtureArgumentSetTestCollection with disableParallelization hard-coded to false, ignoring the [CollectionDefinition(DisableParallelization = true)] attribute on test classes like ReindexJobTests. This caused long-running integration tests that share a single SQL fixture to run concurrently and hang on resource contention. Now the discoverer reads CollectionDefinitionAttribute.DisableParallelization from the test class and propagates it to the test collection created for each fixture-argument variant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
xunit v3 defaults parallelizeTestCollections to true. Integration tests share SQL fixtures and hang when collections run concurrently (ReindexJobTests). Match the E2E config by setting parallelizeTestCollections: false. Reverts the speculative DisableParallelization plumbing in the custom discoverer (the custom discoverer creates one collection per method, so DisableParallelization on a single collection had no effect). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…xunit v2 behavior In xUnit v2, all methods of a test class belong to a single test collection and therefore run serially with a shared IClassFixture lifetime. The v3 rewrite of CustomXunitTestFrameworkDiscoverer accidentally created a new FixtureArgumentSetTestCollection for every (method, variant) combination, which caused xUnit v3's scheduler to run methods of the same class in parallel. This broke tests like ReindexJobTests which rely on per-class serialization and shared fixture state, causing intermittent hangs on the SQL integration test pipeline stages. Cache collection/class instances per (class, variant) inside FindTestsForType so methods of the same test class + variant share the same collection instance, matching v2 semantics. Revert the over-broad parallelizeTestCollections=false workaround from xunit.runner.json; cross-class parallelism is restored. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…llelism In xunit v3 test collections are the unit of parallelism: methods within a collection run serially, collections run in parallel. The previous discoverer used SharedPerVariant as the default, which collapsed every class with the same fixture argument variant (for example [FhirStorageTestsFixtureArgumentSets(DataStore.SqlServer)]) into one collection. All SqlServer integration tests across all classes then ran serially, causing R4/R4B SQL test stages to exceed the 60 minute timeout. Always scope the collection identity to the test class so each (class, variant) pair is its own collection. This matches xunit v2 behavior: one IClassFixture per class, methods within a class run serially, and classes run in parallel. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Linux unit-test 'Publish unit test results' task was the only publish step still using failTaskOnFailedTests: true. MTP's --retry-failed-tests records the original (pre-retry) failure in the trx, so a retried-and-passed test still failed the publish step even though dotnet test succeeded. Match the workaround used by every other PublishTestResults task in the repo (coverage, e2e, sql, cosmos, export). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5507 +/- ##
=======================================
Coverage ? 77.02%
=======================================
Files ? 982
Lines ? 35812
Branches ? 5396
=======================================
Hits ? 27583
Misses ? 6906
Partials ? 1323 🚀 New features to boost your workflow:
|
Address correctness and minimality issues from PR review comments for xUnit3 migration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor fixture argument handling loops to use explicit LINQ Select and Where calls as requested by the PR review comments.\n\nCo-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
3 participants
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.
Description
Describe the changes in this PR.
Related issues
AB#189958
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)