fix(apphost): CI null-ref, package upgrades, namespace cleanup, test reorg#449
Merged
mpaulosky merged 6 commits intoJul 20, 2026
Merged
Conversation
…y and security concerns
…reorg Closes #448 - CRITICAL: ClearCommandAppFixture.DisposeAsync — null-safe App?.DisposeAsync() (unconditional call caused NullReferenceException in CI where App=null!) - Package upgrades in Directory.Packages.props: Auth0.AspNetCore.Authentication 1.8.0 → 1.9.0 Auth0.ManagementApi 8.6.0 → 9.0.0 MongoDB.Driver/Bson 3.9.0 → 3.10.0 NSubstitute 5.3.0 → 6.0.0 OpenTelemetry suite 1.16.0 → 1.17.0 Microsoft.NET.Test.Sdk 18.7.0 → 18.8.1 Microsoft.Extensions.Http.Resilience/ServiceDiscovery 10.7 → 10.8 - AppHost.cs: namespace AppHost (was MyBlog.AppHost); startup resource ordering - MongoDbResourceBuilderExtensions.cs: partial class, bodyless LoggerMessage methods - AppHost.Tests.csproj: RootNamespace=AppHost - Test files moved: Tests/Auth/, Tests/Layout/, Tests/Pages/ → Auth/, Layout/, Pages/ - Copyright headers: 10 files corrected IssueManager → MyBlog - AspireManager retry log corrected: '/3' → '/5' - Web.Tests.Bunit: NSubstitute 6 null-safe Arg.Is predicates + TestSender helper - Directory.Packages.props: trailing newline restored - decisions.md: Decision 5 (AppHost.Tests CI-Safety Pattern) - docs/build-log.txt: verification entries Build: 0 errors, 0 warnings Tests: 557/557 passed (61/61 AppHost.Tests on warm DCP) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t patterns - Documents ClearCommandAppFixture rules (no pre-warm, StopAsync before Dispose, WaitForResourceAsync inside retry boundary) - Documents BasePlaywrightTests.RetryOnNetworkChangedAsync pattern - Documents ThemeToggleTestRuntime trustworthy-state probe pattern - Corrects 'No Playwright project exists' claim — AppHost.Tests now ships E2E coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
squad
Contributor
🏗️ PR Added to Squad Triage QueueThis PR has been labeled with Next steps:
|
Contributor
Test Results Summary556 tests ±0 518 ✅ ±0 13s ⏱️ ±0s Results for commit 8ab5c92. ± Comparison against base commit 197a636. This pull request removes 128 and adds 128 tests. Note that renamed tests count towards both.This pull request removes 38 skipped tests and adds 38 skipped tests. Note that renamed tests count towards both. |
There was a problem hiding this comment.
Pull request overview
This PR stabilizes the AppHost/Playwright test infrastructure for CI safety, modernizes dependencies, and normalizes namespaces/test organization across the solution to reduce build/test friction.
Changes:
- Fixes AppHost test fixture lifecycle to avoid CI null-ref and improve Aspire/MongoDB startup reliability (retry + stop-before-dispose + timeouts).
- Upgrades multiple NuGet packages (Auth0, MongoDB, NSubstitute, OpenTelemetry, MS test SDK, etc.) and updates bUnit tests accordingly (null-safe predicates + deterministic
TestSender). - Cleans up namespaces/RootNamespace settings and refreshes internal documentation/logs.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Web.Tests.Bunit/Testing/TestSender.cs | Adds deterministic ISender test double to avoid NSubstitute 6 behavior collisions in mixed query/command flows. |
| tests/Web.Tests.Bunit/Features/ObjectIdWorkflowTests.cs | Switches some scenarios to TestSender; updates assertions to ReceivedCount and keeps null-safe Arg.Is predicates. |
| tests/Web.Tests.Bunit/Features/EditCategoryRegressionTests.cs | Migrates selected tests to TestSender; adjusts send assertions to ReceivedCount. |
| tests/Web.Tests.Bunit/Features/EditAclTests.cs | Uses TestSender for deterministic MediatR behavior in bUnit tests and registers required responses. |
| tests/Web.Tests.Bunit/Components/RazorSmokeTests.cs | Replaces NSubstitute sender usage with TestSender in several flows; updates send assertions accordingly. |
| tests/Domain.Tests/Entities/CategoryTests.cs | Normalizes test namespace to match new RootNamespace convention. |
| tests/Domain.Tests/Entities/BlogPostTests.cs | Normalizes test namespace to match new RootNamespace convention. |
| tests/Domain.Tests/Entities/BlogPostCategoryTests.cs | Normalizes test namespace to match new RootNamespace convention. |
| tests/Domain.Tests/Domain.Tests.csproj | Changes RootNamespace to Domain to align with updated test namespaces. |
| tests/Domain.Tests/Behaviors/ValidationBehaviorTests.cs | Normalizes test namespace to match new RootNamespace convention. |
| tests/Domain.Tests/Abstractions/ResultTests.cs | Normalizes test namespace to match new RootNamespace convention. |
| tests/AppHost.Tests/WebPlaywrightTests.cs | Updates namespace/imports and corrects header/doc text for MyBlog naming. |
| tests/AppHost.Tests/Pages/NotFoundPageTests.cs | Updates namespace/imports and corrects header naming. |
| tests/AppHost.Tests/Pages/HomePageTests.cs | Updates namespace/imports and corrects header naming. |
| tests/AppHost.Tests/MongoShowStatsIntegrationTests.cs | Updates namespace/imports to the new AppHost test layout. |
| tests/AppHost.Tests/MongoSeedDataIntegrationTests.cs | Improves readiness wait behavior/timeouts and updates namespace/import aliasing. |
| tests/AppHost.Tests/MongoDbStatsCommandTests.cs | Updates namespace to match RootNamespace/layout cleanup. |
| tests/AppHost.Tests/MongoDbSeedCommandTests.cs | Updates namespace to match RootNamespace/layout cleanup. |
| tests/AppHost.Tests/MongoDbContainerConfigurationTests.cs | Updates namespace to match RootNamespace/layout cleanup. |
| tests/AppHost.Tests/MongoDbClearCommandTests.cs | Updates namespace to match RootNamespace/layout cleanup. |
| tests/AppHost.Tests/MongoClearDataIntegrationTests.cs | Updates namespace/imports to the new AppHost test layout. |
| tests/AppHost.Tests/Layout/ThemeToggleTestRuntime.cs | Updates namespace to the new Layout folder convention. |
| tests/AppHost.Tests/Layout/ThemeToggleInteractionTests.cs | Updates namespace/imports to the new Layout folder convention. |
| tests/AppHost.Tests/Layout/LayoutThemeToggleTests.cs | Updates namespace/imports to the new Layout folder convention. |
| tests/AppHost.Tests/Layout/LayoutAuthenticatedTests.cs | Fixes a vacuous assertion and updates namespace/imports. |
| tests/AppHost.Tests/Layout/LayoutAnonymousTests.cs | Updates namespace/imports to the new Layout folder convention. |
| tests/AppHost.Tests/Infrastructure/SkipInCITheoryAttribute.cs | Moves infra namespace from AppHost.Tests.Infrastructure to AppHost.Infrastructure. |
| tests/AppHost.Tests/Infrastructure/SkipInCIFactAttribute.cs | Moves infra namespace from AppHost.Tests.Infrastructure to AppHost.Infrastructure. |
| tests/AppHost.Tests/Infrastructure/PlaywrightManager.cs | Updates namespace and naming/header consistency. |
| tests/AppHost.Tests/Infrastructure/MongoStatsIntegrationCollection.cs | Moves infra namespace to AppHost.Infrastructure. |
| tests/AppHost.Tests/Infrastructure/MongoSeedIntegrationCollection.cs | Moves infra namespace to AppHost.Infrastructure. |
| tests/AppHost.Tests/Infrastructure/MongoClearIntegrationCollection.cs | Moves infra namespace to AppHost.Infrastructure. |
| tests/AppHost.Tests/Infrastructure/ClearCommandAppFixture.cs | Removes DCP pre-warm, adds robust retry/cleanup, moves Mongo wait inside retry, and ensures stop-before-dispose. |
| tests/AppHost.Tests/Infrastructure/AspireManager.cs | Increases retries/timeouts, adds jitter, and improves HttpClient timeout usage and logging accuracy. |
| tests/AppHost.Tests/Infrastructure/AppHostTestCollection.cs | Updates namespace and header naming consistency. |
| tests/AppHost.Tests/EnvVarTests.cs | Updates namespace/header naming consistency. |
| tests/AppHost.Tests/BasePlaywrightTests.cs | Adds transient ERR_NETWORK_CHANGED retry wrapper and improves HttpClient timeout handling. |
| tests/AppHost.Tests/Auth/LoginFallbackTests.cs | Updates namespace to new Auth folder convention and imports to AppHost.Infrastructure. |
| tests/AppHost.Tests/AppHostStartupSmokeTests.cs | Improves HttpClient timeout handling and updates namespace/imports. |
| tests/AppHost.Tests/AppHost.Tests.csproj | Adds RootNamespace to match new namespace layout. |
| src/Web/package-lock.json | Bumps markdownlint-cli2 version in lockfile. |
| src/AppHost/MongoDbResourceBuilderExtensions.cs | Moves to AppHost namespace; minor comment punctuation cleanup. |
| src/AppHost/AppHost.csproj | Updates Aspire AppHost SDK version. |
| src/AppHost/AppHost.cs | Updates namespace usage and resource reference/wait ordering. |
| global.json | Bumps .NET SDK patch version. |
| docs/build-log.txt | Adds a detailed build repair log documenting the changes and verification. |
| Directory.Packages.props | Upgrades core dependency versions and adjusts transitive pins (AngleSharp, etc.). |
| .squad/skills/webapp-testing/SKILL.md | Updates the repo’s testing skill documentation for bUnit + Playwright layers (but still contains stale statements/paths). |
| .squad/decisions/inbox/boromir-release-board-selection.md | Removes an inbox decision entry (likely merged upstream into decisions.md previously). |
| .squad/decisions.md | Adds Decision 5 documenting CI-safety patterns for fixtures and AppHost test strategy. |
Files not reviewed (1)
- src/Web/package-lock.json: Generated file
Comments suppressed due to low confidence (1)
.squad/skills/webapp-testing/SKILL.md:85
- This section still references
tests/Unit.Tests(which doesn’t exist in this repo) and also states that no Playwright/browser-test project exists, which contradicts the earlier bullets in the same doc (and the repo’stests/AppHost.Tests). This makes the skill misleading for contributors.
- `tests/Unit.Tests/Features/UserManagement/ProfileTests.cs` — Profile component claim assertions
- bUnit tests use `BunitContext` (base class from bUnit; test-specific helpers in `TestAuthorizationService.cs`)
- **No** Playwright or dedicated browser-test project exists today.
Comment on lines
+202
to
+203
| - Fix: Changed to null-safe pattern: await (App?.DisposeAsync() ?? ValueTask.CompletedTask) | ||
| (matches the safe pattern already used in AspireManager.DisposeAsync) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #449 +/- ##
==========================================
+ Coverage 76.73% 76.78% +0.05%
==========================================
Files 71 71
Lines 1775 1775
Branches 214 214
==========================================
+ Hits 1362 1363 +1
Misses 336 336
+ Partials 77 76 -1
🚀 New features to boost your workflow:
|
mpaulosky
deleted the
squad/448-fix-apphost-ci-null-ref-and-package-upgrades
branch
July 20, 2026 21:38
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.
Summary
Closes #448
Fan-out review of
src/AppHost/andtests/AppHost.Tests/. All uncommitted working-tree changes staged, verified, and committed.Working as Boromir (DevOps/Infra)
Critical Fix
ClearCommandAppFixture.DisposeAsync— NullReferenceException in CIWhen
CI=true,InitializeAsyncreturns early leavingApp = null!. The previous code calledApp.DisposeAsync()unconditionally, causing aNullReferenceExceptionthat masked the intended CI skip.Fix: Null-safe pattern:
Package Upgrades (Directory.Packages.props)
Other Changes
AppHost; startup resource ordering fixedRootNamespaceTests/Auth/,Tests/Layout/,Tests/Pages/→Auth/,Layout/,Pages/Verification
All 7 pre-push gates passed — 557/557 tests, 0 failures: