Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 15, 2025

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description

This PR migrates the SourceGen unit tests from NUnit to xUnit to maintain consistency with other test projects in the repository, specifically the BindingSourceGen.UnitTests which already uses xUnit.

Changes

Project Configuration

  • Updated SourceGen.UnitTests.csproj to replace NUnit packages with xUnit packages
  • Removed: NUnit (3.13.3) and NUnit3TestAdapter (4.5.0)
  • Added: xunit ($(XunitPackageVersion)) and coverlet.collector ($(CoverletCollectorPackageVersion))

Test Migration

Converted 7 unit tests across 3 test classes:

  • SourceGenXamlTests.cs: 5 tests for XAML code-behind generation
  • SourceGenCssTests.cs: 2 tests for CSS resource generation
  • SourceGenTestsBase.cs: Shared test verification methods

Assertion Updates

  • [Test][Fact]
  • Assert.IsTrue(condition)Assert.True(condition)
  • Assert.IsFalse(condition)Assert.False(condition)
  • Assert.IsTrue(str.Contains(...))Assert.Contains(..., str, StringComparison)
  • Assert.IsFalse(str.Contains(...))Assert.DoesNotContain(..., str, StringComparison)
  • Assert.AreEqual(expected, actual)Assert.Equal(expected, actual)
  • Assert.AreNotEqual(expected, actual)Assert.NotEqual(expected, actual)
  • Updated using statements from NUnit.Framework to Xunit

Testing

All 7 unit tests pass successfully with xUnit:

  • ✅ TestCodeBehindGenerator_BasicXaml
  • ✅ TestCodeBehindGenerator_LocalXaml
  • ✅ TestCodeBehindGenerator_CompilationClone
  • ✅ TestCodeBehindGenerator_ReferenceAdded
  • ✅ TestCodeBehindGenerator_ModifiedXaml
  • ✅ TestCodeBehindGenerator_BasicCss
  • ✅ TestCodeBehindGenerator_ModifiedCss

Benefits

  • Consistency: Aligns with the existing xUnit usage in BindingSourceGen.UnitTests
  • Modernization: xUnit provides better parallel execution support and more expressive assertions
  • Maintainability: Single testing framework reduces maintenance overhead and learning curve for contributors
Original prompt

Migrate SourceGen unit tests from NUnit to xUnit. This should include converting all NUnit test attributes and assertions to their xUnit equivalents, updating test project references from NUnit packages to xUnit packages, and ensuring that all tests continue to pass with the xUnit framework.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Migrate SourceGen unit tests from NUnit to xUnit Migrate SourceGen unit tests from NUnit to xUnit Oct 15, 2025
Copilot finished work on behalf of rmarinho October 15, 2025 18:32
Copilot AI requested a review from rmarinho October 15, 2025 18:32
@rmarinho
Copy link
Member

/azp run MAUI-public

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho rmarinho marked this pull request as ready for review October 18, 2025 10:46
Copilot AI review requested due to automatic review settings October 18, 2025 10:46
@rmarinho rmarinho added this to the .NET 10 SR1 milestone Oct 18, 2025
Copy link
Contributor

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

Migrates SourceGen unit tests from NUnit to xUnit for consistency with other test projects and updates assertions accordingly.

  • Replaced NUnit attributes and assertions with xUnit equivalents.
  • Updated test project package references from NUnit/NUnit3TestAdapter to xUnit and added code coverage collector.
  • Adjusted assertion patterns (e.g., Contains, Equal) across XAML and CSS source generator tests.

Reviewed Changes

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

File Description
SourceGenXamlTests.cs Converted NUnit tests to xUnit with updated assertions for XAML generator scenarios.
SourceGenTestsBase.cs Migrated shared assertion helper to xUnit, removing NUnit usage.
SourceGenCssTests.cs Converted CSS-related generator tests from NUnit to xUnit with updated assertions.
SourceGen.UnitTests.csproj Replaced NUnit dependencies with xUnit and added coverlet collector for test coverage.

Comment on lines +27 to +31
<PackageReference Include="xunit" Version="$(XunitPackageVersion)" />
<PackageReference Include="coverlet.collector" Version="$(CoverletCollectorPackageVersion)">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

The NUnit3TestAdapter was removed, but no xUnit test adapter (xunit.runner.visualstudio) was added. Without it, tests may not be discovered/run in IDEs and some CI configurations. Add the adapter: <PackageReference Include="xunit.runner.visualstudio" Version="$(XunitRunnerVisualStudioPackageVersion)" PrivateAssets="all" />.

Copilot uses AI. Check for mistakes.
{
var actualReason = result2.TrackedSteps[expected.Key].Single().Outputs.Single().Reason;
Assert.AreEqual(expected.Value, actualReason, message: expected.Key);
Assert.Equal(expected.Value, actualReason);
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

[nitpick] The original NUnit assertion included a message (expected.Key) for identifying which step failed; removing it reduces diagnosability. Use Assert.True(expected.Value == actualReason, expected.Key) to preserve the contextual message, or log the key before asserting.

Suggested change
Assert.Equal(expected.Value, actualReason);
Assert.Equal(expected.Value, actualReason, expected.Key);

Copilot uses AI. Check for mistakes.

Assert.IsTrue(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Assert.AreEqual(output1, output2);
Assert.True(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

[nitpick] Using Assert.True with a complex All(...) expression obscures which step failed. Prefer Assert.All for clearer failure reporting: Assert.All(result1.TrackedSteps, kvp => Assert.Equal(IncrementalStepRunReason.New, kvp.Value.Single().Outputs.Single().Reason)); (Same pattern also appears later in this file on similar assertions.)

Suggested change
Assert.True(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Assert.All(result1.TrackedSteps, kvp => Assert.Equal(IncrementalStepRunReason.New, kvp.Value.Single().Outputs.Single().Reason));

Copilot uses AI. Check for mistakes.

Assert.IsTrue(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Assert.AreEqual(output1, output2);
Assert.True(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

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

[nitpick] Asserting All(...) via Assert.True hides which tracked step failed. Replace with Assert.All(result1.TrackedSteps, kvp => Assert.Equal(IncrementalStepRunReason.New, kvp.Value.Single().Outputs.Single().Reason)); for better failure diagnostics.

Suggested change
Assert.True(result1.TrackedSteps.All(s => s.Value.Single().Outputs.Single().Reason == IncrementalStepRunReason.New));
Assert.All(result1.TrackedSteps, kvp => Assert.Equal(IncrementalStepRunReason.New, kvp.Value.Single().Outputs.Single().Reason));

Copilot uses AI. Check for mistakes.
@rmarinho rmarinho merged commit 0ea6ee1 into main Oct 18, 2025
17 checks passed
@rmarinho rmarinho deleted the copilot/migrate-unit-tests-to-xunit branch October 18, 2025 17:59
@rmarinho
Copy link
Member

/backport to net10.0

@github-actions
Copy link
Contributor

Started backporting to net10.0: https://github.com/dotnet/maui/actions/runs/18619055966

@github-actions
Copy link
Contributor

@rmarinho backporting to "net10.0" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Creating an empty commit: Initial plan
Applying: Migrate SourceGen unit tests from NUnit to xUnit
Using index info to reconstruct a base tree...
M	src/Controls/tests/SourceGen.UnitTests/SourceGen.UnitTests.csproj
M	src/Controls/tests/SourceGen.UnitTests/SourceGenCssTests.cs
A	src/Controls/tests/SourceGen.UnitTests/SourceGenXamlTests.cs
Falling back to patching base and 3-way merge...
Auto-merging src/Controls/tests/SourceGen.UnitTests/SourceGen.UnitTests.csproj
CONFLICT (content): Merge conflict in src/Controls/tests/SourceGen.UnitTests/SourceGen.UnitTests.csproj
Auto-merging src/Controls/tests/SourceGen.UnitTests/SourceGenCssTests.cs
CONFLICT (content): Merge conflict in src/Controls/tests/SourceGen.UnitTests/SourceGenCssTests.cs
CONFLICT (modify/delete): src/Controls/tests/SourceGen.UnitTests/SourceGenXamlTests.cs deleted in HEAD and modified in Migrate SourceGen unit tests from NUnit to xUnit.  Version Migrate SourceGen unit tests from NUnit to xUnit of src/Controls/tests/SourceGen.UnitTests/SourceGenXamlTests.cs left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0002 Migrate SourceGen unit tests from NUnit to xUnit
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@StephaneDelcroix
Copy link
Contributor

what was the reason for this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants