Dependencies: Revert NUnit 4 upgrade to unblock integration tests#22562
Merged
Conversation
…latest major versions (#22155)" This reverts commit 7014f9a on v18/dev to resolve the Part3Of4 SQL Server integration test nightly hangs that started around 2026-04-10. Root cause was confirmed by hang-dump analysis: a sync-over-async call in ContentCacheRefresher.HandleMemoryCache (.GetAwaiter().GetResult() on an async cache method) that NUnit 3's pumping synchronization context had been quietly completing on the test thread. NUnit 4 dropped that behaviour, so the continuation now requires a free thread-pool thread; under CI conditions it deadlocks. Reverting #22155 on a branch was verified to make the nightly pass. This is a temporary rollback to unblock v18; the proper fix is to make ContentCacheRefresher.Refresh async end-to-end, tracked separately. Additional adjustments beyond the pure revert to keep the branch compiling: - CoreConfigurationHttpTests.cs: added `using Umbraco.Cms.Core.Services;` for IUserService (referenced by post-#22155 code unaffected by the revert). - ContentVersionCleanupServiceTest.cs: merged imports so both AutoFixture.NUnit3 (from revert) and Microsoft.Extensions.Options (from unrelated later commit) stay. - UdiTests.cs and ContentPermissionResourceTests.cs: removed unused `using NUnit.Framework.Legacy;` (namespace introduced in NUnit 4). - BackOfficeAuthorizationInitializationMiddlewareTests.cs: replaced `[CancelAfter(5000)]` with its NUnit 3 equivalent `[Timeout(5000)]`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @andr317c's task in 3m 46s —— View job Review complete
Review posted. One suggestion noted around the remaining sync-over-async in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Target: origin/v18/dev · Based on commit: f4df7a78 · Classified as: Test-only
Reverts the NUnit 4 upgrade (PR #22155) back to NUnit 3.14.0 across all test projects to unblock deterministic Windows SQL Server LocalDB integration test hangs caused by a sync-over-async deadlock in ContentCacheRefresher.HandleMemoryCache that NUnit 4's thread-pool changes exposed.
Suggestions
tests/Umbraco.Tests.Integration/TestServerTest/UmbracoWebApplicationFactory.cs:30:ClearHost()uses_host.StopAsync().GetAwaiter().GetResult()— sync-over-async, but safe under NUnit 3 teardown (no problematic SynchronizationContext). The previous NUnit 4 workaround wrapped this inTask.Run(); reverting to a directGetAwaiter().GetResult()is correct here. The underlying pattern remains technical debt, consistent with the PR description's note that the real fix is makingContentCacheRefresherasync end-to-end.
Approved
Clean, mechanically consistent revert. All 44 files show the expected mechanical changes: AutoFixture.NUnit4 → AutoFixture.NUnit3 namespace swaps, [CancelAfter] → [Timeout] attribute replacements, CancellationToken parameter removals, and assertion style rollbacks from NUnit 4 fluent constraints to NUnit 3 CollectionAssert equivalents — all correctly applied and consistently ordered (expected-before-actual throughout). The UmbracoWebApplicationFactory simplification (dropping Task.Run wrapping in favour of _host.Start()) and the CoreConfigurationHttpTests factory refactor (removing NUnit 4-specific AddTestServices/ModelsBuilder workarounds) restore the known-good pre-upgrade state. Root cause is clearly documented in the PR description.
This looks good to be merged as-is, but please do a manual sanity check and testing before merging.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reverts the NUnit 4.x upgrade (and related test dependency changes) back to NUnit 3.x in order to unblock deterministically hanging Windows SQL Server LocalDB integration tests.
Changes:
- Downgrade NUnit + NUnit3TestAdapter and switch AutoFixture integration back to
AutoFixture.NUnit3. - Revert NUnit 4-only test APIs/usages (e.g.,
CancelAfter, constraint.AsCollection) back to NUnit 3-compatible assertions/attributes. - Adjust test-server bootstrapping to remove NUnit 4-specific deadlock workarounds.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Umbraco.Tests.UnitTests/Umbraco.Web.Common/FileNameTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoPasswordHasherTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGeneratorTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/PublishedContentQueryTests.cs | Replace NUnit4 constraint collection assertions with NUnit3 CollectionAssert. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/TaskHelperTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/DefaultContentVersionCleanupPolicyTest.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/ContentVersionCleanupServiceTest.cs | Switch AutoFixture integration back to NUnit3 (and add required using). |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/ContentNavigationServiceBaseTests.cs | Replace NUnit4 collection constraints with NUnit3 CollectionAssert. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Security/UserGroupAssignmentAuthorizationTests.cs | Replace NUnit4 equivalence constraint with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Security/ContentPermissionResourceTests.cs | Remove NUnit legacy namespace usage; use CollectionAssert for collections. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Scheduling/ContentVersionCleanupTest.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/ContentFinderByPageIdQueryTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Routing/ContentFinderByKeyTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DateTimeDataValueEditorSortableTests.cs | Replace NUnit4 .AsCollection assertions with CollectionAssert.AreEqual. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/PropertyEditors/DataValueReferenceFactoryCollectionTests.cs | Replace NUnit4 collection constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/Extensions/UmbracoBuilderExtensionsTests.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/EnumerableExtensionsTests.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Core/CoreThings/UdiTests.cs | Remove NUnit legacy namespace usage. |
| tests/Umbraco.Tests.UnitTests/Umbraco.Cms.Api.Management/Middleware/BackOfficeAuthorizationInitializationMiddlewareTests.cs | Replace CancelAfter with NUnit3 Timeout. |
| tests/Umbraco.Tests.UnitTests/AutoFixture/InlineAutoMoqDataAttribute.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.UnitTests/AutoFixture/AutoMoqDataAttribute.cs | Switch AutoFixture integration back to NUnit3. |
| tests/Umbraco.Tests.Integration/Umbraco.Persistence.EFCore/Scoping/EFCoreLockTests.cs | Replace CancelAfter with Timeout and remove NUnit4 CancellationToken injection pattern. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs | Replace Is.Empty constraint with CollectionAssert.IsEmpty. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MemberServiceTests.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaServiceTests.cs | Replace CancelAfter with Timeout; use NUnit3-friendly empty assertions and remove CancellationToken injection pattern. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/MediaListViewServiceTests.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ElementEditingServiceTests.Restore.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ElementEditingServiceTests.Move.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentSearchServiceTests.cs | Replace .AsCollection constraints with CollectionAssert.AreEqual. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentListViewServiceTests.cs | Replace collection constraints with CollectionAssert variants. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/AuditServiceTests.cs | Replace item-not-null constraint with CollectionAssert.AllItemsAreNotNull. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/PublicAccessRepositoryTest.cs | Replace equivalence constraint with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs | Replace CancelAfter with Timeout and remove NUnit4 CancellationToken injection pattern. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Packaging/CreatedPackageSchemaTests.cs | Replace Not.Empty/Unique constraints with CollectionAssert equivalents. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserGroupServiceTests.cs | Add ShortStringHelper and replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.Update.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/MediaNavigationServiceTests.Rebuild.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DynamicRootServiceTests.cs | Replace Has.Member constraints with CollectionAssert.Contains. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Update.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/Umbraco.Core/Services/DocumentNavigationServiceTests.Rebuild.cs | Replace equivalence constraints with CollectionAssert.AreEquivalent. |
| tests/Umbraco.Tests.Integration/TestServerTest/UmbracoWebApplicationFactory.cs | Remove NUnit4 deadlock workaround and revert to synchronous host start/stop patterns. |
| tests/Umbraco.Tests.Integration/TestServerTest/CoreConfigurationHttpTests.cs | Rework factory setup to set content root via WithWebHostBuilder/ConfigureTestServices. |
| tests/Umbraco.Tests.Common/Umbraco.Tests.Common.csproj | Swap AutoFixture NUnit integration package back to NUnit3. |
| tests/Directory.Packages.props | Downgrade NUnit + adapter versions and swap AutoFixture NUnit integration package back to NUnit3. |
lauraneto
added
release/no-notes
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
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.
Reverts #22155 (NUnit 4 upgrade). Integration tests on Windows SQL Server LocalDB have been deterministically hanging on v18/dev. The hang dump points at a sync-over-async call in
ContentCacheRefresher.HandleMemoryCache.GetAwaiter().GetResult()on an async cache method.We don't fully understand why NUnit 4 makes this hang, only that it does, reverting to NUnit 3 unblocks CI. Best guess is something about how NUnit 4 awaits async tests changes the thread-pool timing enough that, under CI memory pressure, the inner continuation never resumes.
The real fix is removing that sync-over-async from
ContentCacheRefresher, but it's a gnarly change and not something we want to tackle right now. Worth keeping in mind next time we look at NUnit 4 or that refresher.