Update NUnit to v4#6287
Conversation
florelis
left a comment
There was a problem hiding this comment.
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)
| 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); |
There was a problem hiding this comment.
Depending on the type of Settings, maybe this could be
Assert.That(output.Settings, Contains.Key(LocalManifestFiles));There was a problem hiding this comment.
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
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
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 alsoNUnit3TestAdapter/Microsoft.ApplicationInsights) insrc/Directory.Packages.props. - Converted E2E tests from classic assertions (
Assert.True/False/AreEqual/Throws*) toAssert.That(..., constraint)patterns compatible with NUnit 4. - Minor test cleanups (e.g., removing unused
usingdirectives, 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. |
| <PackageReference Include="Moq" /> | ||
| <PackageReference Include="System.Text.Json" /> | ||
| <PackageReference Include="xunit" /> |
|
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>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
This looks fun . . . Will trace down the root cause when I get home. However . . . looks like the tests themselves are passing, so that's promising at least |
📖 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.propschange 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
src/AppInstallerCLIE2ETests.✅ Checklist
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow