Skip to content

Update NUnit to v4#6287

Open
Trenly wants to merge 11 commits into
microsoft:masterfrom
Trenly:NUnitUpdate
Open

Update NUnit to v4#6287
Trenly wants to merge 11 commits into
microsoft:masterfrom
Trenly:NUnitUpdate

Conversation

@Trenly

@Trenly Trenly commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Upgrade the AppInstallerCLIE2ETests suite to NUnit 4 and modernize the supporting test code to the NUnit 4 constraint model. This lands on the latest available NUnit 4.x release in NuGet (4.6.1)

The update converts the E2E suite off NUnit 3 classic assertions, fixes the delegate-based assertion overloads required by NUnit 4, and carries the package version change through the affected test projects that use centralized package management.

I opted for full migration instead of the simpler Directory.Build.props change as I expect this will be a one-time migration and I generally believe that using the newer format will be more performant in the future (plus I had the extra tokens to burn, so why not)

🔗 References

🔍 Validation

  • Search-based verification confirmed the old NUnit 3 classic assertions were removed from src/AppInstallerCLIE2ETests.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

@Trenly Trenly marked this pull request as ready for review June 16, 2026 03:03
@Trenly Trenly requested a review from a team as a code owner June 16, 2026 03:03

@florelis florelis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would be good to use some of the native constraints in more places. Like
Assert.That(x, Is.EqualTo(0)) -> Assert.That(x, Is.Zero)
Assert.That(s.Contains("substr"), Is.True) -> Assert.That(s, Contains.Substring("substr"))
Assert.That(File.Exists(f), Is.True) -> Assert.That(f, Does.Exist)

Comment thread src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilDownload.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/ConfigureProcessorPathCommand.cs Outdated
Assert.IsFalse(output.Settings.ContainsKey(NotAnAdminSettingName));
Assert.That(output, Is.Not.Null);
Assert.That(output.Settings, Is.Not.Null);
Assert.That(output.Settings.ContainsKey(LocalManifestFiles), Is.True);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Depending on the type of Settings, maybe this could be

            Assert.That(output.Settings, Contains.Key(LocalManifestFiles));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It could be, but I wanted to keep all the tests consistent with how they handle JSON. I'm happy to update across all the files if that is preferred, but don't see a lot of benefit in that. Of course, it may also be easier to take that as a follow on to minimize the amount of time needed for a re-review on this PR

Comment thread src/AppInstallerCLIE2ETests/DSCv3SourceResourceCommand.cs Outdated
florelis
florelis previously approved these changes Jul 14, 2026
@florelis florelis requested a review from Copilot July 14, 2026 18:52
@florelis

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Upgrades the AppInstallerCLIE2ETests suite to NUnit 4 and migrates assertions throughout the E2E test codebase to the NUnit constraint model, while also updating centrally-managed NuGet package versions.

Changes:

  • Updated central package versions for nunit (and also NUnit3TestAdapter / Microsoft.ApplicationInsights) in src/Directory.Packages.props.
  • Converted E2E tests from classic assertions (Assert.True/False/AreEqual/Throws*) to Assert.That(..., constraint) patterns compatible with NUnit 4.
  • Minor test cleanups (e.g., removing unused using directives, resolving type name ambiguity via alias).

Reviewed changes

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

Show a summary per file
File Description
src/WinGetUtilInterop.UnitTests/WinGetUtilInterop.UnitTests.csproj Adds a package reference (currently appears unnecessary).
src/Microsoft.Management.Configuration.UnitTests/Tests/ConfigurationProcessorTelemetryTests.cs Adds a TelemetryEvent using-alias to disambiguate types.
src/Directory.Packages.props Updates centrally-managed package versions (includes NUnit and other packages).
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilSQLiteIndex.cs Migrates WinGetUtil SQLite index tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilManifest.cs Migrates WinGetUtil manifest tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilLog.cs Migrates logging test assertions to constraint-based style.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilInstallerMetadataCollection.cs Migrates installer metadata tests to constraint-based assertions (incl. exception assertions).
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilDownload.cs Removes unused System.Linq and migrates assertions to constraints.
src/AppInstallerCLIE2ETests/WinGetUtil/WinGetUtilCompareVersions.cs Migrates version compare test assertion to constraints.
src/AppInstallerCLIE2ETests/ValidateCommand.cs Migrates validate command tests to constraint-based assertions (incl. non-ASCII manifest filename).
src/AppInstallerCLIE2ETests/UpgradeCommand.cs Migrates upgrade command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/UninstallCommand.cs Migrates uninstall command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ShowCommand.cs Migrates show command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/SetUpFixture.cs Migrates setup fixture assertions to constraint-based style.
src/AppInstallerCLIE2ETests/SearchCommand.cs Migrates search command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ResumeCommand.cs Migrates resume tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/RepairCommand.cs Migrates repair command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Pinning.cs Migrates pinning behavior tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ListCommand.cs Migrates list command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/UpgradeInterop.cs Migrates interop upgrade tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/UninstallInterop.cs Migrates interop uninstall tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/Shutdown.cs Migrates shutdown interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/RepairInterop.cs Migrates repair interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/PackageCatalogInterop.cs Migrates package catalog interop tests (incl. async exception assertions) to constraint style.
src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs Migrates group policy interop tests; updates exception assertion patterns.
src/AppInstallerCLIE2ETests/Interop/FindPackagesInterop.cs Migrates find-packages interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/DownloadInterop.cs Migrates download interop tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/ConnectionValidationInterop.cs Migrates connection validation tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Interop/BaseInterop.cs Migrates base interop assertion(s); file header encoding changed.
src/AppInstallerCLIE2ETests/InprocTestbedTests.cs Migrates in-proc testbed exit-code assertion to constraints.
src/AppInstallerCLIE2ETests/ImportCommand.cs Migrates import command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/Helpers/TestCommon.cs Migrates shared E2E helper assertions to constraint-based style.
src/AppInstallerCLIE2ETests/HashCommand.cs Migrates hash command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/GroupPolicy.cs Migrates CLI group policy tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/FontCommand.cs Migrates font command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/FeaturesCommand.cs Migrates features command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ErrorCommand.cs Migrates error command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3UserSettingsFileResourceCommand.cs Migrates DSCv3 user settings resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3SourceResourceCommand.cs Migrates DSCv3 source resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3ResourceTestBase.cs Migrates DSCv3 shared test utilities to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3PackageResourceCommand.cs Migrates DSCv3 package resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DSCv3AdminSettingsResourceCommand.cs Migrates DSCv3 admin settings resource tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/DownloadCommand.cs Migrates download command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureTestCommand.cs Migrates configure test command validations to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureShowCommand.cs Migrates configure show command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureProcessorPathCommand.cs Migrates configure processor-path tests and regex assertion usage to constraints.
src/AppInstallerCLIE2ETests/ConfigureListCommand.cs Migrates configure list command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/ConfigureExportCommand.cs Migrates configure export command tests to constraint-based assertions.
src/AppInstallerCLIE2ETests/AppShutdownTests.cs Migrates app shutdown tests to constraint-based assertions.
doc/ReleaseNotes.md Updates release notes with NUnit v4 migration entry and bumps “New in” header.

Comment thread src/AppInstallerCLIE2ETests/Interop/BaseInterop.cs Outdated
Comment thread src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs
Comment thread src/AppInstallerCLIE2ETests/Interop/GroupPolicyForInterop.cs
Comment on lines 13 to 15
<PackageReference Include="Moq" />
<PackageReference Include="System.Text.Json" />
<PackageReference Include="xunit" />
Comment thread src/Directory.Packages.props
Comment thread src/Directory.Packages.props
@florelis

Copy link
Copy Markdown
Member

I don't think any of the Copilot comments need to be resolved. The BOM issue I would want to have fixed, but I think that would be a repo-level thing, like the normalization of line endings we did.

@Trenly

Trenly commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

I don't think any of the Copilot comments need to be resolved. The BOM issue I would want to have fixed, but I think that would be a repo-level thing, like the normalization of line endings we did.

Reading through them, I agree. If the pipelines fail, I'll address the two that I've left open. I've replied to the others with my justification for resolving them

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@florelis

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@Trenly

Trenly commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

This looks fun . . .

##[error]TearDown failed for test fixture AppInstallerCLIE2ETests.SetUpFixture
##[error]One or more child tests were ignored
##[error]TearDown : System.IO.IOException : The file 'C:\Users\VssAdministrator\AppData\Local\Temp\E2ETestLogs\Unpackaged\WinGetCOM-2026-07-14-21-20-31.670.log' already exists.
##[error]StackTrace: --TearDown
##[error]   at System.IO.FileSystem.CopyFile(String sourceFullPath, String destFullPath, Boolean overwrite)
##[error]   at AppInstallerCLIE2ETests.Helpers.TestCommon.CopyDirectory(String sourceDirName, String destDirName) in D:\a\1\s\src\AppInstallerCLIE2ETests\Helpers\TestCommon.cs:line 1070
##[error]   at AppInstallerCLIE2ETests.Helpers.TestCommon.PublishE2ETestLogs() in D:\a\1\s\src\AppInstallerCLIE2ETests\Helpers\TestCommon.cs:line 545
##[error]   at AppInstallerCLIE2ETests.SetUpFixture.TearDown() in D:\a\1\s\src\AppInstallerCLIE2ETests\SetUpFixture.cs:line 84
##[error]   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
##[error]   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Will trace down the root cause when I get home. However . . . looks like the tests themselves are passing, so that's promising at least

Total tests: 100
     Passed: 94
    Skipped: 6

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.

3 participants