From 685c57c46d6d95abafc7215b92438f956d8a1d2b Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Wed, 13 May 2026 02:12:06 -0700 Subject: [PATCH 01/17] Migrate integration tests to TUnit.AspNetCore WebApplicationTest - Add TUnit.AspNetCore 1.40.5 package reference - Create IntegrationTestBase : WebApplicationTest for per-test factory isolation, InServiceScope helpers, and trace correlation - Rewrite WebApplicationFactory to extend TestWebApplicationFactory (removes IAsyncInitializer, moves to ConcurrentBag for multi-connection tracking) - Migrate all test classes to inherit from IntegrationTestBase: - Remove [NotInParallel] and [ClassDataSource] attributes - Collapse 6 McpRateLimitingTests classes into 1 (fresh DI per test = fresh rate limiter) - Update McpTestHelper to accept TracedWebApplicationFactory - Use Factory.Inner.CreateClient(options) where AllowAutoRedirect=false is needed - Remove TUnit.Core.Interfaces import (IAsyncInitializer no longer used) --- Directory.Packages.props | 1 + .../ContentRateLimitingTests.cs | 8 +-- .../EssentialCSharp.Web.Tests.csproj | 1 + EssentialCSharp.Web.Tests/FunctionalTests.cs | 12 ++-- .../IntegrationTestBase.cs | 19 ++++++ .../ListingSourceCodeControllerTests.cs | 13 ++-- .../McpApiTokenServiceTests.cs | 11 ++-- .../McpRateLimitingTests.cs | 62 ++++++------------ EssentialCSharp.Web.Tests/McpTestHelper.cs | 16 ++--- EssentialCSharp.Web.Tests/McpTests.cs | 37 +++++------ .../McpToolContractTests.cs | 11 ++-- .../RouteConfigurationServiceTests.cs | 14 +---- .../SitemapXmlHelpersTests.cs | 28 +++------ .../WebApplicationFactory.cs | 63 ++++++------------- 14 files changed, 117 insertions(+), 179 deletions(-) create mode 100644 EssentialCSharp.Web.Tests/IntegrationTestBase.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index c484ba53..992d7a61 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -23,6 +23,7 @@ + diff --git a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs index cdbb0de4..5d475d53 100644 --- a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs +++ b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs @@ -5,17 +5,15 @@ namespace EssentialCSharp.Web.Tests; /// /// HTTP integration tests for the "content" rate limit policy. -/// Uses its own factory (PerClass) to get a fresh in-memory rate limiter for each run. +/// Each test gets its own factory (fresh IHost) so the rate limiter starts from a clean state. /// Anonymous users are limited to 10 requests per minute on chapter content pages. /// -[ClassDataSource(Shared = SharedType.PerClass)] -public class ContentRateLimitingTests(WebApplicationFactory factory) +public class ContentRateLimitingTests : IntegrationTestBase { [Test] public async Task ContentEndpoint_ExceedingPerMinuteLimit_Returns429() { - // AllowAutoRedirect = false prevents redirect-following from consuming extra permits. - HttpClient client = factory.CreateClient(new WebApplicationFactoryClientOptions + HttpClient client = Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); diff --git a/EssentialCSharp.Web.Tests/EssentialCSharp.Web.Tests.csproj b/EssentialCSharp.Web.Tests/EssentialCSharp.Web.Tests.csproj index 3d3fb91e..23a0c5ff 100644 --- a/EssentialCSharp.Web.Tests/EssentialCSharp.Web.Tests.csproj +++ b/EssentialCSharp.Web.Tests/EssentialCSharp.Web.Tests.csproj @@ -13,6 +13,7 @@ + diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index 31aa723e..1bb392c4 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -2,9 +2,7 @@ namespace EssentialCSharp.Web.Tests; -[NotInParallel("FunctionalTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class FunctionalTests(WebApplicationFactory factory) +public class FunctionalTests : IntegrationTestBase { [Test] [Arguments("/")] @@ -15,7 +13,7 @@ public class FunctionalTests(WebApplicationFactory factory) [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -31,7 +29,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -47,9 +45,9 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); using HttpResponseMessage response = await client.GetAsync("/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); } -} \ No newline at end of file +} diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs new file mode 100644 index 00000000..26fa9109 --- /dev/null +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -0,0 +1,19 @@ +using Microsoft.Extensions.DependencyInjection; +using TUnit.AspNetCore; + +namespace EssentialCSharp.Web.Tests; + +public abstract class IntegrationTestBase : WebApplicationTest +{ + public T InServiceScope(Func action) + { + using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); + return action(scope.ServiceProvider); + } + + public void InServiceScope(Action action) + { + using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); + action(scope.ServiceProvider); + } +} diff --git a/EssentialCSharp.Web.Tests/ListingSourceCodeControllerTests.cs b/EssentialCSharp.Web.Tests/ListingSourceCodeControllerTests.cs index 05a29baa..7232a7c3 100644 --- a/EssentialCSharp.Web.Tests/ListingSourceCodeControllerTests.cs +++ b/EssentialCSharp.Web.Tests/ListingSourceCodeControllerTests.cs @@ -4,14 +4,13 @@ namespace EssentialCSharp.Web.Tests; -[ClassDataSource(Shared = SharedType.PerClass)] -public class ListingSourceCodeControllerTests(WebApplicationFactory factory) +public class ListingSourceCodeControllerTests : IntegrationTestBase { [Test] public async Task GetListing_WithValidChapterAndListing_Returns200WithContent() { // Arrange - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); // Act using HttpResponseMessage response = await client.GetAsync("/api/ListingSourceCode/chapter/1/listing/1"); @@ -35,7 +34,7 @@ public async Task GetListing_WithValidChapterAndListing_Returns200WithContent() public async Task GetListing_WithInvalidChapter_Returns404() { // Arrange - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); // Act using HttpResponseMessage response = await client.GetAsync("/api/ListingSourceCode/chapter/999/listing/1"); @@ -48,7 +47,7 @@ public async Task GetListing_WithInvalidChapter_Returns404() public async Task GetListing_WithInvalidListing_Returns404() { // Arrange - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); // Act using HttpResponseMessage response = await client.GetAsync("/api/ListingSourceCode/chapter/1/listing/999"); @@ -61,7 +60,7 @@ public async Task GetListing_WithInvalidListing_Returns404() public async Task GetListingsByChapter_WithValidChapter_ReturnsMultipleListings() { // Arrange - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); // Act using HttpResponseMessage response = await client.GetAsync("/api/ListingSourceCode/chapter/1"); @@ -92,7 +91,7 @@ public async Task GetListingsByChapter_WithValidChapter_ReturnsMultipleListings( public async Task GetListingsByChapter_WithInvalidChapter_ReturnsEmptyList() { // Arrange - HttpClient client = factory.CreateClient(); + HttpClient client = Factory.CreateClient(); // Act using HttpResponseMessage response = await client.GetAsync("/api/ListingSourceCode/chapter/999"); diff --git a/EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs b/EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs index 8e78d718..0eb417bd 100644 --- a/EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs +++ b/EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs @@ -1,13 +1,10 @@ using EssentialCSharp.Web.Models; using EssentialCSharp.Web.Services; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; namespace EssentialCSharp.Web.Tests; -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpApiTokenServiceTests(WebApplicationFactory factory) +public class McpApiTokenServiceTests : IntegrationTestBase { private readonly List _scopes = []; @@ -21,8 +18,8 @@ public void DisposeScopes() private async Task<(string UserId, McpApiTokenService TokenService)> ArrangeAsync(string prefix) { - string userId = await McpTestHelper.CreateUserAsync(factory, prefix); - var scope = factory.Services.CreateScope(); + string userId = await McpTestHelper.CreateUserAsync(Factory, prefix); + var scope = Factory.Services.CreateScope(); _scopes.Add(scope); var tokenService = scope.ServiceProvider.GetRequiredService(); return (userId, tokenService); @@ -30,7 +27,7 @@ public void DisposeScopes() private async Task FillToLimitAsync(string userId) { - var scope = factory.Services.CreateScope(); + var scope = Factory.Services.CreateScope(); _scopes.Add(scope); var tokenService = scope.ServiceProvider.GetRequiredService(); for (int i = 0; i < McpApiTokenService.MaxTokensPerUser; i++) diff --git a/EssentialCSharp.Web.Tests/McpRateLimitingTests.cs b/EssentialCSharp.Web.Tests/McpRateLimitingTests.cs index a89be156..202f0a56 100644 --- a/EssentialCSharp.Web.Tests/McpRateLimitingTests.cs +++ b/EssentialCSharp.Web.Tests/McpRateLimitingTests.cs @@ -2,27 +2,25 @@ using System.Text.Json; using EssentialCSharp.Web.Data; using EssentialCSharp.Web.Services; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; namespace EssentialCSharp.Web.Tests; /// -/// Each class gets its own factory so the global limiter starts from a fresh state. +/// Each test method gets its own per-test factory (fresh IHost + rate limiter state) +/// via TUnit.AspNetCore's WebApplicationTest, so [NotInParallel] is no longer needed. /// -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpDistinctUserRateLimitingTests(WebApplicationFactory factory) +public class McpRateLimitingTests : IntegrationTestBase { [Test] public async Task DistinctValidMcpUsers_DoNotShareRateLimitBucket() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); for (int i = 0; i < 31; i++) { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, $"mcp-rate-limit-isolation-{i}", userPrefix: $"mcp-isolation-{i}"); @@ -35,20 +33,15 @@ await Assert.That(response.StatusCode) .Because($"distinct MCP user request {i + 1} should use its own rate-limit bucket"); } } -} -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpPerUserRateLimitingTests(WebApplicationFactory factory) -{ [Test] public async Task SingleValidMcpUser_ExceedingTokenBucket_Returns429AndDoesNotCountRejectedRequests() { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, "mcp-rate-limit-single-user", userPrefix: "mcp-single-user"); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); List statuses = []; string? rateLimitedPayload = null; string? rateLimitedContentType = null; @@ -70,7 +63,7 @@ public async Task SingleValidMcpUser_ExceedingTokenBucket_Returns429AndDoesNotCo } } - (long UsageCount, bool HasLastUsedAt) tokenUsage = factory.InServiceScope(services => + (long UsageCount, bool HasLastUsedAt) tokenUsage = InServiceScope(services => { var db = services.GetRequiredService(); byte[] tokenHash = McpApiTokenService.HashToken(rawToken); @@ -104,16 +97,11 @@ await Assert.That(statuses.Skip(McpRateLimiterPolicy.AuthenticatedTokenLimit) await Assert.That(error.GetProperty("code").GetInt32()).IsEqualTo(-32000); await Assert.That(error.GetProperty("message").GetString()).Contains("Rate limit exceeded"); } -} -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpAnonymousRateLimitingTests(WebApplicationFactory factory) -{ [Test] public async Task InvalidMcpBearerRequests_FallBackToAnonymousIpBucket() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); for (int i = 0; i < McpRateLimiterPolicy.AnonymousPermitLimit; i++) { @@ -132,21 +120,16 @@ await Assert.That(response.StatusCode) using HttpResponseMessage rateLimitedResponse = await client.SendAsync(rateLimitedRequest); await Assert.That(rateLimitedResponse.StatusCode).IsEqualTo(HttpStatusCode.TooManyRequests); } -} -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpCookieIsolationRateLimitingTests(WebApplicationFactory factory) -{ [Test] public async Task InvalidMcpBearerRequests_WithDifferentSiteCookies_StillShareAnonymousIpBucket() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); for (int i = 0; i < McpRateLimiterPolicy.AnonymousPermitLimit; i++) { - string cookieUserId = await McpTestHelper.CreateUserAsync(factory, $"mcp-cookie-user-{i}"); - (string cookieName, string cookieValue) = await McpTestHelper.CreateIdentityApplicationCookieAsync(factory, cookieUserId); + string cookieUserId = await McpTestHelper.CreateUserAsync(Factory, $"mcp-cookie-user-{i}"); + (string cookieName, string cookieValue) = await McpTestHelper.CreateIdentityApplicationCookieAsync(Factory, cookieUserId); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(request, "mcp_invalid_token_that_does_not_exist"); @@ -158,8 +141,8 @@ await Assert.That(response.StatusCode) .Because($"invalid MCP bearer request {i + 1} should ignore the site cookie principal and stay in the anonymous/IP bucket"); } - string finalCookieUserId = await McpTestHelper.CreateUserAsync(factory, "mcp-cookie-user-final"); - (string finalCookieName, string finalCookieValue) = await McpTestHelper.CreateIdentityApplicationCookieAsync(factory, finalCookieUserId); + string finalCookieUserId = await McpTestHelper.CreateUserAsync(Factory, "mcp-cookie-user-final"); + (string finalCookieName, string finalCookieValue) = await McpTestHelper.CreateIdentityApplicationCookieAsync(Factory, finalCookieUserId); using var rateLimitedRequest = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(rateLimitedRequest, "mcp_invalid_token_that_does_not_exist"); @@ -168,20 +151,15 @@ await Assert.That(response.StatusCode) using HttpResponseMessage rateLimitedResponse = await client.SendAsync(rateLimitedRequest); await Assert.That(rateLimitedResponse.StatusCode).IsEqualTo(HttpStatusCode.TooManyRequests); } -} -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpGlobalBypassRateLimitingTests(WebApplicationFactory factory) -{ [Test] public async Task ValidMcpPostRequests_DoNotConsumeGlobalLimiterBudgetForGetShim() { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, "mcp-global-bypass", userPrefix: "mcp-bypass"); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); for (int i = 0; i < 10; i++) { @@ -211,16 +189,11 @@ await Assert.That(getResponse.StatusCode) using HttpResponseMessage rateLimitedGetResponse = await client.SendAsync(rateLimitedGetRequest); await Assert.That(rateLimitedGetResponse.StatusCode).IsEqualTo(HttpStatusCode.TooManyRequests); } -} -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpWellKnownIsolationRateLimitingTests(WebApplicationFactory factory) -{ [Test] public async Task WellKnownRequests_DoNotConsumeContentLimiterBudget() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); for (int i = 0; i < 10; i++) { @@ -243,3 +216,4 @@ await Assert.That(contentResponse.StatusCode) await Assert.That(rateLimitedResponse.StatusCode).IsEqualTo(HttpStatusCode.TooManyRequests); } } + diff --git a/EssentialCSharp.Web.Tests/McpTestHelper.cs b/EssentialCSharp.Web.Tests/McpTestHelper.cs index 12ed4255..a1f66911 100644 --- a/EssentialCSharp.Web.Tests/McpTestHelper.cs +++ b/EssentialCSharp.Web.Tests/McpTestHelper.cs @@ -9,15 +9,17 @@ using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using TUnit.AspNetCore; namespace EssentialCSharp.Web.Tests; internal static class McpTestHelper { - public static HttpClient CreateClient(WebApplicationFactory factory) => factory.CreateClient(new WebApplicationFactoryClientOptions - { - AllowAutoRedirect = false - }); + public static HttpClient CreateClient(TracedWebApplicationFactory factory) => + factory.Inner.CreateClient(new WebApplicationFactoryClientOptions + { + AllowAutoRedirect = false + }); public static HttpRequestMessage CreateInitializeRequest(string path = "/mcp") { @@ -50,7 +52,7 @@ public static void AddBearerToken(HttpRequestMessage request, string rawToken) = public static void AddCookie(HttpRequestMessage request, string cookieName, string cookieValue) => request.Headers.Add("Cookie", $"{cookieName}={cookieValue}"); - public static async Task CreateUserAsync(WebApplicationFactory factory, string userPrefix) + public static async Task CreateUserAsync(TracedWebApplicationFactory factory, string userPrefix) { string userId = Guid.NewGuid().ToString(); string suffix = Guid.NewGuid().ToString("N")[..8]; @@ -73,7 +75,7 @@ public static async Task CreateUserAsync(WebApplicationFactory factory, } public static async Task<(string UserId, string RawToken)> CreateUserAndTokenAsync( - WebApplicationFactory factory, + TracedWebApplicationFactory factory, string tokenName, string userPrefix = "mcp-test", DateTime? expiresAt = null) @@ -90,7 +92,7 @@ public static async Task CreateUserAsync(WebApplicationFactory factory, } public static async Task<(string CookieName, string CookieValue)> CreateIdentityApplicationCookieAsync( - WebApplicationFactory factory, + TracedWebApplicationFactory factory, string userId) { using var scope = factory.Services.CreateScope(); diff --git a/EssentialCSharp.Web.Tests/McpTests.cs b/EssentialCSharp.Web.Tests/McpTests.cs index 109b1f10..a4f4d5ed 100644 --- a/EssentialCSharp.Web.Tests/McpTests.cs +++ b/EssentialCSharp.Web.Tests/McpTests.cs @@ -1,19 +1,16 @@ using System.Net; using System.Text; using EssentialCSharp.Web.Services; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; namespace EssentialCSharp.Web.Tests; -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpTests(WebApplicationFactory factory) +public class McpTests : IntegrationTestBase { [Test] public async Task McpTokenEndpoint_WithoutAuth_Returns401() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using HttpResponseMessage response = await client.PostAsync("/api/McpToken", null); @@ -23,7 +20,7 @@ public async Task McpTokenEndpoint_WithoutAuth_Returns401() [Test] public async Task McpEndpoint_WithoutToken_Returns401() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); using HttpResponseMessage response = await client.SendAsync(request); @@ -34,11 +31,11 @@ public async Task McpEndpoint_WithoutToken_Returns401() [Test] public async Task McpEndpoint_WithSiteCookieButWithoutBearer_Returns401() { - string cookieUserId = await McpTestHelper.CreateUserAsync(factory, "mcp-cookie-only"); + string cookieUserId = await McpTestHelper.CreateUserAsync(Factory, "mcp-cookie-only"); (string cookieName, string cookieValue) = - await McpTestHelper.CreateIdentityApplicationCookieAsync(factory, cookieUserId); + await McpTestHelper.CreateIdentityApplicationCookieAsync(Factory, cookieUserId); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddCookie(request, cookieName, cookieValue); @@ -51,11 +48,11 @@ public async Task McpEndpoint_WithSiteCookieButWithoutBearer_Returns401() public async Task McpEndpoint_WithValidToken_Returns200AndListsTools() { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, "integration-test", userPrefix: "mcp-testuser"); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); // Step 1: Initialize the MCP session using var initRequest = McpTestHelper.CreateInitializeRequest("/mcp"); @@ -108,7 +105,7 @@ public async Task McpEndpoint_WithValidToken_Returns200AndListsTools() [Test] public async Task McpEndpoint_WithInvalidToken_Returns401() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(request, "mcp_invalid_token_that_does_not_exist"); using HttpResponseMessage response = await client.SendAsync(request); @@ -118,16 +115,16 @@ public async Task McpEndpoint_WithInvalidToken_Returns401() [Test] public async Task McpEndpoint_WithRevokedToken_Returns401() { - string testUserId = await McpTestHelper.CreateUserAsync(factory, "revoked-user"); + string testUserId = await McpTestHelper.CreateUserAsync(Factory, "revoked-user"); string rawToken; - using (var scope = factory.Services.CreateScope()) + using (var scope = Factory.Services.CreateScope()) { var tokenService = scope.ServiceProvider.GetRequiredService(); (rawToken, var entity) = await tokenService.CreateTokenAsync(testUserId, "revoke-test"); await tokenService.RevokeTokenAsync(entity.Id, testUserId); } - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(request, rawToken); using HttpResponseMessage response = await client.SendAsync(request); @@ -138,12 +135,12 @@ public async Task McpEndpoint_WithRevokedToken_Returns401() public async Task McpEndpoint_WithExpiredToken_Returns401() { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, "expired-test", userPrefix: "expired-user", expiresAt: DateTime.UtcNow.AddSeconds(-1)); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(request, rawToken); using HttpResponseMessage response = await client.SendAsync(request); @@ -153,7 +150,7 @@ public async Task McpEndpoint_WithExpiredToken_Returns401() [Test] public async Task WellKnownOAuthProtectedResource_AllMethodsReturn404WithoutRedirectAndNoStore() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); foreach (HttpMethod method in new[] { HttpMethod.Get, HttpMethod.Post, HttpMethod.Options }) { @@ -171,7 +168,7 @@ await Assert.That(response.StatusCode) [Test] public async Task McpEndpoint_PreflightFromLoopbackOrigin_ReturnsCorsHeaders() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = new HttpRequestMessage(HttpMethod.Options, "/mcp"); request.Headers.Add("Origin", "http://localhost:6274"); request.Headers.Add("Access-Control-Request-Method", "POST"); @@ -191,7 +188,7 @@ public async Task McpEndpoint_PreflightFromLoopbackOrigin_ReturnsCorsHeaders() [Test] public async Task McpEndpoint_GetFromLoopbackOrigin_Returns405WithoutRedirect() { - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var request = new HttpRequestMessage(HttpMethod.Get, "/mcp"); request.Headers.Add("Origin", "http://localhost:6274"); request.Headers.Accept.ParseAdd("text/event-stream"); diff --git a/EssentialCSharp.Web.Tests/McpToolContractTests.cs b/EssentialCSharp.Web.Tests/McpToolContractTests.cs index ec806220..d9e1582f 100644 --- a/EssentialCSharp.Web.Tests/McpToolContractTests.cs +++ b/EssentialCSharp.Web.Tests/McpToolContractTests.cs @@ -2,15 +2,12 @@ using System.Text; using System.Text.Json; using EssentialCSharp.Web.Services; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; namespace EssentialCSharp.Web.Tests; -[NotInParallel("McpTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class McpToolContractTests(WebApplicationFactory factory) +public class McpToolContractTests : IntegrationTestBase { [Test] public async Task McpToolsList_StructuredAndHybridTools_AdvertiseOutputSchema() @@ -201,10 +198,10 @@ public async Task McpCall_GetChapterSections_WithInvalidChapter_ReturnsMcpError( private async Task<(HttpClient Client, string RawToken, string? SessionId)> CreateAuthenticatedSessionAsync() { (_, string rawToken) = await McpTestHelper.CreateUserAndTokenAsync( - factory, + Factory, "mcp-contract-test", userPrefix: "mcp-contract"); - HttpClient client = McpTestHelper.CreateClient(factory); + HttpClient client = McpTestHelper.CreateClient(Factory); using var initRequest = McpTestHelper.CreateInitializeRequest("/mcp"); McpTestHelper.AddBearerToken(initRequest, rawToken); @@ -223,7 +220,7 @@ public async Task McpCall_GetChapterSections_WithInvalidChapter_ReturnsMcpError( private string GetConfiguredBaseUrl() { - string baseUrl = factory.Services.GetRequiredService>().Value.BaseUrl; + string baseUrl = Factory.Services.GetRequiredService>().Value.BaseUrl; return baseUrl.TrimEnd('/') + "/"; } diff --git a/EssentialCSharp.Web.Tests/RouteConfigurationServiceTests.cs b/EssentialCSharp.Web.Tests/RouteConfigurationServiceTests.cs index ed5866fc..a238451d 100644 --- a/EssentialCSharp.Web.Tests/RouteConfigurationServiceTests.cs +++ b/EssentialCSharp.Web.Tests/RouteConfigurationServiceTests.cs @@ -3,21 +3,13 @@ namespace EssentialCSharp.Web.Tests; -[ClassDataSource(Shared = SharedType.PerClass)] -public class RouteConfigurationServiceTests +public class RouteConfigurationServiceTests : IntegrationTestBase { - private readonly WebApplicationFactory _Factory; - - public RouteConfigurationServiceTests(WebApplicationFactory factory) - { - _Factory = factory; - } - [Test] public async Task GetStaticRoutes_ShouldReturnExpectedRoutes() { // Act - var routes = _Factory.InServiceScope(serviceProvider => + var routes = InServiceScope(serviceProvider => { var routeConfigurationService = serviceProvider.GetRequiredService(); return routeConfigurationService.GetStaticRoutes().ToList(); @@ -33,4 +25,4 @@ public async Task GetStaticRoutes_ShouldReturnExpectedRoutes() await Assert.That(routes).Contains("announcements"); await Assert.That(routes).Contains("termsofservice"); } -} \ No newline at end of file +} diff --git a/EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs b/EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs index 1adc0dff..b47b49bf 100644 --- a/EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs +++ b/EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs @@ -3,20 +3,10 @@ using EssentialCSharp.Web.Helpers; using EssentialCSharp.Web.Services; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; namespace EssentialCSharp.Web.Tests; -[NotInParallel("SitemapTests")] -[ClassDataSource(Shared = SharedType.PerClass)] -public class SitemapXmlHelpersTests +public class SitemapXmlHelpersTests : IntegrationTestBase { - private readonly WebApplicationFactory _Factory; - - public SitemapXmlHelpersTests(WebApplicationFactory factory) - { - _Factory = factory; - } - [Test] public async Task EnsureSitemapHealthy_WithValidSiteMappings_DoesNotThrow() { @@ -73,7 +63,7 @@ public async Task GenerateSitemapXml_DoesNotIncludeIdentityRoutes() var baseUrl = "https://test.example.com/"; // Act & Assert - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -99,7 +89,7 @@ public async Task GenerateSitemapXml_IncludesBaseUrl() var baseUrl = "https://test.example.com/"; // Act & Assert - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -131,7 +121,7 @@ public async Task GenerateSitemapXml_IncludesSiteMappingsMarkedForXml() }; // Act & Assert - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -153,7 +143,7 @@ public async Task GenerateSitemapXml_DoesNotIncludeIndexRoutes() var baseUrl = "https://test.example.com/"; // Act & Assert - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -174,7 +164,7 @@ public async Task GenerateSitemapXml_DoesNotIncludeErrorRoutes() var baseUrl = "https://test.example.com/"; // Act & Assert - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -195,7 +185,7 @@ public async Task GenerateSitemapXml_DoesNotIncludeSitemapRoute() var baseUrl = "https://test.example.com/"; // Act - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -221,7 +211,7 @@ public async Task GenerateSitemapXml_UsesLastModifiedDateFromSiteMapping() }; // Act - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, @@ -244,7 +234,7 @@ public async Task GenerateSitemapXml_DoesNotSetLastModifiedDateWhenSiteMappingDa }; // Act - var routeConfigurationService = _Factory.Services.GetRequiredService(); + var routeConfigurationService = Factory.Services.GetRequiredService(); SitemapXmlHelpers.GenerateSitemapXml( siteMappings, routeConfigurationService, diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 287150ee..2e0af30b 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -1,9 +1,9 @@ +using System.Collections.Concurrent; using System.Data.Common; using EssentialCSharp.Web.Data; using EssentialCSharp.Web.Services; -using TUnit.Core.Interfaces; +using TUnit.AspNetCore; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Data.Sqlite; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; @@ -12,19 +12,12 @@ namespace EssentialCSharp.Web.Tests; -public sealed class WebApplicationFactory : WebApplicationFactory, IAsyncInitializer +public sealed class WebApplicationFactory : TestWebApplicationFactory { - public Task InitializeAsync() - { - // Force eager server initialization before tests run. - // This is thread-safe and prevents race conditions from parallel tests - // calling CreateClient() concurrently during lazy init. - _ = Server; - return Task.CompletedTask; - } - private static string SqlConnectionString => $"DataSource=file:{Guid.NewGuid()}?mode=memory&cache=shared"; - private SqliteConnection? _Connection; + + // Each per-test factory's ConfigureWebHost creates a new connection; track all for disposal. + private readonly ConcurrentBag _connections = []; protected override void ConfigureWebHost(IWebHostBuilder builder) { @@ -58,12 +51,15 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) services.Remove(migrationServiceDescriptor); } - _Connection = new SqliteConnection(SqlConnectionString); - _Connection.Open(); + // Capture in a local variable so each per-test factory's closure binds + // to its own connection, not to a shared field that gets overwritten. + SqliteConnection connection = new(SqlConnectionString); + connection.Open(); + _connections.Add(connection); services.AddDbContext(options => { - options.UseSqlite(_Connection); + options.UseSqlite(connection); }); using ServiceProvider serviceProvider = services.BuildServiceProvider(); @@ -80,37 +76,12 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); } - /// - /// Executes an action within a service scope, handling scope creation and cleanup automatically. - /// - /// The return type of the action - /// The action to execute with the scoped service provider - /// The result of the action - public T InServiceScope(Func action) - { - var factory = Services.GetRequiredService(); - using var scope = factory.CreateScope(); - return action(scope.ServiceProvider); - } - - /// - /// Executes an action within a service scope, handling scope creation and cleanup automatically. - /// - /// The action to execute with the scoped service provider - public void InServiceScope(Action action) - { - var factory = Services.GetRequiredService(); - using var scope = factory.CreateScope(); - action(scope.ServiceProvider); - } - public override async ValueTask DisposeAsync() { await base.DisposeAsync().ConfigureAwait(false); - if (_Connection != null) + foreach (SqliteConnection connection in _connections) { - await _Connection.DisposeAsync().ConfigureAwait(false); - _Connection = null; + await connection.DisposeAsync().ConfigureAwait(false); } GC.SuppressFinalize(this); } @@ -120,8 +91,10 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); if (disposing) { - _Connection?.Dispose(); - _Connection = null; + foreach (SqliteConnection connection in _connections) + { + connection.Dispose(); + } } } } From 10bfa0565ee350a0b66192272587de8a6befa666 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Wed, 13 May 2026 08:53:52 -0700 Subject: [PATCH 02/17] fix: use Factory.Inner.CreateClient() in FunctionalTests for redirect following TracedWebApplicationFactory.CreateClient() uses CreateDefaultClient() which sets AllowAutoRedirect=false. Tests expecting 200/404 after app-level redirects (e.g. / -> first chapter) need Factory.Inner.CreateClient() which defaults to AllowAutoRedirect=true. --- EssentialCSharp.Web.Tests/FunctionalTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index 1bb392c4..6342dea4 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,7 +13,7 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - HttpClient client = Factory.CreateClient(); + HttpClient client = Factory.Inner.CreateClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -29,7 +29,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - HttpClient client = Factory.CreateClient(); + HttpClient client = Factory.Inner.CreateClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -45,7 +45,7 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - HttpClient client = Factory.CreateClient(); + HttpClient client = Factory.Inner.CreateClient(); using HttpResponseMessage response = await client.GetAsync("/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); From 756a93de7ee186c527fa078ff56421e8cf548374 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Wed, 13 May 2026 09:06:40 -0700 Subject: [PATCH 03/17] refactor(tests): restore trace correlation and add async InServiceScope helpers - McpTestHelper.CreateClient: factory.Inner.CreateClient(options) -> factory.CreateClient() TracedWebApplicationFactory.CreateClient() already uses AllowAutoRedirect=false; going through Inner was silently bypassing activity propagation for all MCP tests. - ContentRateLimitingTests: same fix - Factory.Inner -> Factory.CreateClient() - FunctionalTests: introduce CreateRedirectFollowingClient() helper on IntegrationTestBase to document the explicit AllowAutoRedirect=true trade-off; all three test methods use it since the app issues redirects even for 404 responses. - IntegrationTestBase: add InServiceScopeAsync and InServiceScopeAsync overloads so callers no longer need to manually manage IServiceScope for async DB/service work. - WebApplicationFactory: add Interlocked disposal guard to prevent double-dispose of SqliteConnection instances when both DisposeAsync and Dispose(bool) are called. --- .../ContentRateLimitingTests.cs | 6 +---- EssentialCSharp.Web.Tests/FunctionalTests.cs | 6 ++--- .../IntegrationTestBase.cs | 23 +++++++++++++++++++ EssentialCSharp.Web.Tests/McpTestHelper.cs | 6 +---- .../WebApplicationFactory.cs | 4 ++++ 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs index 5d475d53..7685eeca 100644 --- a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs +++ b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs @@ -1,5 +1,4 @@ using System.Net; -using Microsoft.AspNetCore.Mvc.Testing; namespace EssentialCSharp.Web.Tests; @@ -13,10 +12,7 @@ public class ContentRateLimitingTests : IntegrationTestBase [Test] public async Task ContentEndpoint_ExceedingPerMinuteLimit_Returns429() { - HttpClient client = Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions - { - AllowAutoRedirect = false - }); + HttpClient client = Factory.CreateClient(); // Anonymous limit is 10/min. First 10 requests should not be rate-limited. for (int i = 0; i < 10; i++) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index 6342dea4..d3fca5d8 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,7 +13,7 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - HttpClient client = Factory.Inner.CreateClient(); + HttpClient client = CreateRedirectFollowingClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -29,7 +29,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - HttpClient client = Factory.Inner.CreateClient(); + HttpClient client = CreateRedirectFollowingClient(); using HttpResponseMessage response = await client.GetAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -45,7 +45,7 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - HttpClient client = Factory.Inner.CreateClient(); + HttpClient client = CreateRedirectFollowingClient(); using HttpResponseMessage response = await client.GetAsync("/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 26fa9109..a7bf21b0 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -1,3 +1,4 @@ +using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using TUnit.AspNetCore; @@ -5,6 +6,16 @@ namespace EssentialCSharp.Web.Tests; public abstract class IntegrationTestBase : WebApplicationTest { + /// + /// Creates an HTTP client with redirect following enabled. + /// NOTE: This bypasses TUnit trace correlation because + /// does not expose a CreateClient(WebApplicationFactoryClientOptions) overload. + /// Use for all + /// other tests where AllowAutoRedirect=false is acceptable. + /// + protected HttpClient CreateRedirectFollowingClient() => + Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = true }); + public T InServiceScope(Func action) { using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); @@ -16,4 +27,16 @@ public void InServiceScope(Action action) using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); action(scope.ServiceProvider); } + + public async Task InServiceScopeAsync(Func> action) + { + using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); + return await action(scope.ServiceProvider); + } + + public async Task InServiceScopeAsync(Func action) + { + using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); + await action(scope.ServiceProvider); + } } diff --git a/EssentialCSharp.Web.Tests/McpTestHelper.cs b/EssentialCSharp.Web.Tests/McpTestHelper.cs index a1f66911..db610ea4 100644 --- a/EssentialCSharp.Web.Tests/McpTestHelper.cs +++ b/EssentialCSharp.Web.Tests/McpTestHelper.cs @@ -6,7 +6,6 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Identity; -using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using TUnit.AspNetCore; @@ -16,10 +15,7 @@ namespace EssentialCSharp.Web.Tests; internal static class McpTestHelper { public static HttpClient CreateClient(TracedWebApplicationFactory factory) => - factory.Inner.CreateClient(new WebApplicationFactoryClientOptions - { - AllowAutoRedirect = false - }); + factory.CreateClient(); public static HttpRequestMessage CreateInitializeRequest(string path = "/mcp") { diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 2e0af30b..3420c94c 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -76,8 +76,11 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); } + private int _disposed; + public override async ValueTask DisposeAsync() { + if (Interlocked.Exchange(ref _disposed, 1) == 1) return; await base.DisposeAsync().ConfigureAwait(false); foreach (SqliteConnection connection in _connections) { @@ -88,6 +91,7 @@ public override async ValueTask DisposeAsync() protected override void Dispose(bool disposing) { + if (Interlocked.Exchange(ref _disposed, 1) == 1) return; base.Dispose(disposing); if (disposing) { From 148b516e282aa8a52a5bb2831e1b55c9913f1b7c Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Wed, 13 May 2026 09:51:28 -0700 Subject: [PATCH 04/17] fix: scope disposal guard to SQLite connections only, not base disposal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared _disposed flag was blocking base.Dispose(bool) when DisposeAsync ran first — the base's async path may call Dispose(true), which our guard intercepted, skipping base resource cleanup. Fix: use a _connectionsDisposed flag that only gates the SQLite connection cleanup (which we own). Base disposal calls are always forwarded. Also removes the redundant GC.SuppressFinalize — the base class handles it. Addresses Copilot PR review comments. --- EssentialCSharp.Web.Tests/WebApplicationFactory.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 3420c94c..db0feeb3 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -76,29 +76,25 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); } - private int _disposed; + private int _connectionsDisposed; public override async ValueTask DisposeAsync() { - if (Interlocked.Exchange(ref _disposed, 1) == 1) return; await base.DisposeAsync().ConfigureAwait(false); - foreach (SqliteConnection connection in _connections) + if (Interlocked.Exchange(ref _connectionsDisposed, 1) == 0) { - await connection.DisposeAsync().ConfigureAwait(false); + foreach (SqliteConnection connection in _connections) + await connection.DisposeAsync().ConfigureAwait(false); } - GC.SuppressFinalize(this); } protected override void Dispose(bool disposing) { - if (Interlocked.Exchange(ref _disposed, 1) == 1) return; base.Dispose(disposing); - if (disposing) + if (disposing && Interlocked.Exchange(ref _connectionsDisposed, 1) == 0) { foreach (SqliteConnection connection in _connections) - { connection.Dispose(); - } } } } From 1a845d8c9fe83984e088d1a6b608a82cd8dfec31 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 15 May 2026 10:45:34 -0700 Subject: [PATCH 05/17] test: fix integration factory disposal and traced redirects --- EssentialCSharp.Web.Tests/FunctionalTests.cs | 9 ++-- .../IntegrationTestBase.cs | 40 +++++++++++++---- .../WebApplicationFactory.cs | 43 +++++-------------- 3 files changed, 45 insertions(+), 47 deletions(-) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index d3fca5d8..4e2b8cdd 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,8 +13,7 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - HttpClient client = CreateRedirectFollowingClient(); - using HttpResponseMessage response = await client.GetAsync(relativeUrl); + using HttpResponseMessage response = await GetWithRedirectsAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); } @@ -29,8 +28,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - HttpClient client = CreateRedirectFollowingClient(); - using HttpResponseMessage response = await client.GetAsync(relativeUrl); + using HttpResponseMessage response = await GetWithRedirectsAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -45,8 +43,7 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - HttpClient client = CreateRedirectFollowingClient(); - using HttpResponseMessage response = await client.GetAsync("/non-existing-page1234"); + using HttpResponseMessage response = await GetWithRedirectsAsync("/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); } diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index a7bf21b0..8bc3ebff 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Mvc.Testing; +using System.Net; using Microsoft.Extensions.DependencyInjection; using TUnit.AspNetCore; @@ -7,14 +7,38 @@ namespace EssentialCSharp.Web.Tests; public abstract class IntegrationTestBase : WebApplicationTest { /// - /// Creates an HTTP client with redirect following enabled. - /// NOTE: This bypasses TUnit trace correlation because - /// does not expose a CreateClient(WebApplicationFactoryClientOptions) overload. - /// Use for all - /// other tests where AllowAutoRedirect=false is acceptable. + /// Executes a GET request and follows redirect responses while preserving + /// TUnit trace correlation by using . /// - protected HttpClient CreateRedirectFollowingClient() => - Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = true }); + protected async Task GetWithRedirectsAsync(string relativeUrl, int maxRedirects = 10) + { + HttpClient client = Factory.CreateClient(); + HttpResponseMessage response = await client.GetAsync(relativeUrl); + + for (int redirectCount = 0; + redirectCount < maxRedirects && IsRedirectStatusCode(response.StatusCode); + redirectCount++) + { + Uri? location = response.Headers.Location; + if (location is null) + { + return response; + } + + response.Dispose(); + + response = await client.GetAsync(location); + } + + return response; + } + + private static bool IsRedirectStatusCode(HttpStatusCode statusCode) => + statusCode == HttpStatusCode.Moved || + statusCode == HttpStatusCode.Found || + statusCode == HttpStatusCode.RedirectMethod || + statusCode == HttpStatusCode.TemporaryRedirect || + statusCode == HttpStatusCode.PermanentRedirect; public T InServiceScope(Func action) { diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index db0feeb3..43bb135e 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -1,4 +1,3 @@ -using System.Collections.Concurrent; using System.Data.Common; using EssentialCSharp.Web.Data; using EssentialCSharp.Web.Services; @@ -16,9 +15,6 @@ public sealed class WebApplicationFactory : TestWebApplicationFactory { private static string SqlConnectionString => $"DataSource=file:{Guid.NewGuid()}?mode=memory&cache=shared"; - // Each per-test factory's ConfigureWebHost creates a new connection; track all for disposal. - private readonly ConcurrentBag _connections = []; - protected override void ConfigureWebHost(IWebHostBuilder builder) { builder.ConfigureServices(services => @@ -55,17 +51,20 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) // to its own connection, not to a shared field that gets overwritten. SqliteConnection connection = new(SqlConnectionString); connection.Open(); - _connections.Add(connection); - services.AddDbContext(options => + services.AddSingleton(connection); + + services.AddDbContext((serviceProvider, options) => { - options.UseSqlite(connection); + DbConnection dbConnection = serviceProvider.GetRequiredService(); + options.UseSqlite(dbConnection); }); - using ServiceProvider serviceProvider = services.BuildServiceProvider(); - using IServiceScope scope = serviceProvider.CreateScope(); - IServiceProvider scopedServices = scope.ServiceProvider; - EssentialCSharpWebContext db = scopedServices.GetRequiredService(); + DbContextOptions dbContextOptions = + new DbContextOptionsBuilder() + .UseSqlite(connection) + .Options; + using EssentialCSharpWebContext db = new(dbContextOptions); db.Database.EnsureCreated(); @@ -75,26 +74,4 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) _ => TestListingSourceCodeServiceHelper.CreateService()); }); } - - private int _connectionsDisposed; - - public override async ValueTask DisposeAsync() - { - await base.DisposeAsync().ConfigureAwait(false); - if (Interlocked.Exchange(ref _connectionsDisposed, 1) == 0) - { - foreach (SqliteConnection connection in _connections) - await connection.DisposeAsync().ConfigureAwait(false); - } - } - - protected override void Dispose(bool disposing) - { - base.Dispose(disposing); - if (disposing && Interlocked.Exchange(ref _connectionsDisposed, 1) == 0) - { - foreach (SqliteConnection connection in _connections) - connection.Dispose(); - } - } } From 2f9c8313438e4cf08af84d85780de92e849b18dc Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Fri, 15 May 2026 10:50:30 -0700 Subject: [PATCH 06/17] test: fix SqliteConnection disposal analyzer warning --- .../WebApplicationFactory.cs | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 43bb135e..e9f8aa56 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -8,6 +8,7 @@ using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; namespace EssentialCSharp.Web.Tests; @@ -47,12 +48,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) services.Remove(migrationServiceDescriptor); } - // Capture in a local variable so each per-test factory's closure binds - // to its own connection, not to a shared field that gets overwritten. - SqliteConnection connection = new(SqlConnectionString); - connection.Open(); - - services.AddSingleton(connection); + services.AddSingleton(_ => CreateOpenSqliteConnection()); services.AddDbContext((serviceProvider, options) => { @@ -60,13 +56,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) options.UseSqlite(dbConnection); }); - DbContextOptions dbContextOptions = - new DbContextOptionsBuilder() - .UseSqlite(connection) - .Options; - using EssentialCSharpWebContext db = new(dbContextOptions); - - db.Database.EnsureCreated(); + services.AddHostedService(); // Replace IListingSourceCodeService with one backed by TestData services.RemoveAll(); @@ -74,4 +64,24 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) _ => TestListingSourceCodeServiceHelper.CreateService()); }); } + + private static SqliteConnection CreateOpenSqliteConnection() + { + SqliteConnection connection = new(SqlConnectionString); + connection.Open(); + return connection; + } + + private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider) : IHostedService + { + public async Task StartAsync(CancellationToken cancellationToken) + { + using IServiceScope scope = serviceProvider.CreateScope(); + EssentialCSharpWebContext dbContext = + scope.ServiceProvider.GetRequiredService(); + await dbContext.Database.EnsureCreatedAsync(cancellationToken); + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + } } From c47b9bec7c41b3c5644d533bd071b3ed972e700e Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 10:54:27 -0700 Subject: [PATCH 07/17] test: address PR feedback on startup order and redirects --- EssentialCSharp.Web.Tests/FunctionalTests.cs | 6 +++--- EssentialCSharp.Web.Tests/IntegrationTestBase.cs | 9 ++++++++- EssentialCSharp.Web.Tests/WebApplicationFactory.cs | 3 ++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index 4e2b8cdd..3e6659fe 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,7 +13,7 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - using HttpResponseMessage response = await GetWithRedirectsAsync(relativeUrl); + using HttpResponseMessage response = await GetFollowingRedirectsAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); } @@ -28,7 +28,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - using HttpResponseMessage response = await GetWithRedirectsAsync(relativeUrl); + using HttpResponseMessage response = await GetFollowingRedirectsAsync(relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -43,7 +43,7 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - using HttpResponseMessage response = await GetWithRedirectsAsync("/non-existing-page1234"); + using HttpResponseMessage response = await GetFollowingRedirectsAsync("/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); } diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 8bc3ebff..330469fc 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -9,8 +9,9 @@ public abstract class IntegrationTestBase : WebApplicationTest /// Executes a GET request and follows redirect responses while preserving /// TUnit trace correlation by using . + /// This helper intentionally follows redirects using GET requests only. /// - protected async Task GetWithRedirectsAsync(string relativeUrl, int maxRedirects = 10) + protected async Task GetFollowingRedirectsAsync(string relativeUrl, int maxRedirects = 10) { HttpClient client = Factory.CreateClient(); HttpResponseMessage response = await client.GetAsync(relativeUrl); @@ -30,6 +31,12 @@ protected async Task GetWithRedirectsAsync(string relativeU response = await client.GetAsync(location); } + if (IsRedirectStatusCode(response.StatusCode)) + { + throw new InvalidOperationException( + $"Exceeded redirect limit ({maxRedirects}) for '{relativeUrl}'. Last status: {(int)response.StatusCode} {response.StatusCode}."); + } + return response; } diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index e9f8aa56..dadbfba0 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -56,7 +56,8 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) options.UseSqlite(dbConnection); }); - services.AddHostedService(); + // Ensure schema exists before any other hosted service that reads from the database. + services.Insert(0, ServiceDescriptor.Singleton()); // Replace IListingSourceCodeService with one backed by TestData services.RemoveAll(); From 93269b396a88bfcf7e132672b66c271084fed299 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 11:45:57 -0700 Subject: [PATCH 08/17] test: address remaining PR review comments --- .../ContentRateLimitingTests.cs | 2 +- EssentialCSharp.Web.Tests/FunctionalTests.cs | 9 +++++--- .../IntegrationTestBase.cs | 10 +++++++-- EssentialCSharp.Web.Tests/McpTestHelper.cs | 3 ++- .../WebApplicationFactory.cs | 22 +++++++++++++++++-- 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs index 7685eeca..8105a4bb 100644 --- a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs +++ b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs @@ -12,7 +12,7 @@ public class ContentRateLimitingTests : IntegrationTestBase [Test] public async Task ContentEndpoint_ExceedingPerMinuteLimit_Returns429() { - HttpClient client = Factory.CreateClient(); + using HttpClient client = CreateClientWithoutRedirectFollowing(); // Anonymous limit is 10/min. First 10 requests should not be rate-limited. for (int i = 0; i < 10; i++) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index 3e6659fe..fa72240a 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,7 +13,8 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - using HttpResponseMessage response = await GetFollowingRedirectsAsync(relativeUrl); + using HttpClient client = Factory.CreateClient(); + using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); } @@ -28,7 +29,8 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - using HttpResponseMessage response = await GetFollowingRedirectsAsync(relativeUrl); + using HttpClient client = Factory.CreateClient(); + using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -43,7 +45,8 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - using HttpResponseMessage response = await GetFollowingRedirectsAsync("/non-existing-page1234"); + using HttpClient client = Factory.CreateClient(); + using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, "/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); } diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 330469fc..3db97999 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -1,4 +1,5 @@ using System.Net; +using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using TUnit.AspNetCore; @@ -11,9 +12,14 @@ public abstract class IntegrationTestBase : WebApplicationTest. /// This helper intentionally follows redirects using GET requests only. /// - protected async Task GetFollowingRedirectsAsync(string relativeUrl, int maxRedirects = 10) + protected HttpClient CreateClientWithoutRedirectFollowing() => + Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + + protected static async Task GetFollowingGetRedirectsAsync( + HttpClient client, + string relativeUrl, + int maxRedirects = 10) { - HttpClient client = Factory.CreateClient(); HttpResponseMessage response = await client.GetAsync(relativeUrl); for (int redirectCount = 0; diff --git a/EssentialCSharp.Web.Tests/McpTestHelper.cs b/EssentialCSharp.Web.Tests/McpTestHelper.cs index db610ea4..e564da4f 100644 --- a/EssentialCSharp.Web.Tests/McpTestHelper.cs +++ b/EssentialCSharp.Web.Tests/McpTestHelper.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; using TUnit.AspNetCore; @@ -15,7 +16,7 @@ namespace EssentialCSharp.Web.Tests; internal static class McpTestHelper { public static HttpClient CreateClient(TracedWebApplicationFactory factory) => - factory.CreateClient(); + factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); public static HttpRequestMessage CreateInitializeRequest(string path = "/mcp") { diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index dadbfba0..feee69d1 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -56,8 +56,26 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) options.UseSqlite(dbConnection); }); - // Ensure schema exists before any other hosted service that reads from the database. - services.Insert(0, ServiceDescriptor.Singleton()); + // Ensure schema exists before any hosted service that reads from the database. + ServiceDescriptor ensureCreatedDescriptor = + ServiceDescriptor.Singleton(); + int firstHostedServiceIndex = -1; + for (int i = 0; i < services.Count; i++) + { + if (services[i].ServiceType == typeof(IHostedService)) + { + firstHostedServiceIndex = i; + break; + } + } + if (firstHostedServiceIndex >= 0) + { + services.Insert(firstHostedServiceIndex, ensureCreatedDescriptor); + } + else + { + services.Add(ensureCreatedDescriptor); + } // Replace IListingSourceCodeService with one backed by TestData services.RemoveAll(); From 395b9d8b5fc28b0c7d6db1e6115b11b6b3d55afc Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 15:50:57 -0700 Subject: [PATCH 09/17] refactor(tests): address PR feedback - rename helper, fix doc, scoped DbConnection - Rename CreateClientWithoutRedirectFollowing -> CreateClientWithoutAutoRedirect (clearer name, avoids double-negative) - Fix XML doc comment on the helper (was copy-pasted from GetFollowingGetRedirectsAsync) - Change DbConnection from singleton to scoped with a keep-alive connection, preventing SQLite 'database is locked' errors under concurrent requests - Add Dispose(bool) override to clean up _keepAliveConnection - Update PR description: correct method name and hosted-service ordering description --- .../ContentRateLimitingTests.cs | 2 +- .../IntegrationTestBase.cs | 8 ++--- .../WebApplicationFactory.cs | 32 +++++++++++++++---- 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs index 8105a4bb..78e0e701 100644 --- a/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs +++ b/EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs @@ -12,7 +12,7 @@ public class ContentRateLimitingTests : IntegrationTestBase [Test] public async Task ContentEndpoint_ExceedingPerMinuteLimit_Returns429() { - using HttpClient client = CreateClientWithoutRedirectFollowing(); + using HttpClient client = CreateClientWithoutAutoRedirect(); // Anonymous limit is 10/min. First 10 requests should not be rate-limited. for (int i = 0; i < 10; i++) diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 3db97999..1f14ba99 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -8,11 +8,11 @@ namespace EssentialCSharp.Web.Tests; public abstract class IntegrationTestBase : WebApplicationTest { /// - /// Executes a GET request and follows redirect responses while preserving - /// TUnit trace correlation by using . - /// This helper intentionally follows redirects using GET requests only. + /// Creates an with AllowAutoRedirect = false so callers can + /// assert exact redirect status codes and Location headers without the client + /// silently following them. /// - protected HttpClient CreateClientWithoutRedirectFollowing() => + protected HttpClient CreateClientWithoutAutoRedirect() => Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); protected static async Task GetFollowingGetRedirectsAsync( diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index feee69d1..9001f487 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -14,7 +14,13 @@ namespace EssentialCSharp.Web.Tests; public sealed class WebApplicationFactory : TestWebApplicationFactory { - private static string SqlConnectionString => $"DataSource=file:{Guid.NewGuid()}?mode=memory&cache=shared"; + // One GUID per factory instance → each factory gets its own isolated in-memory database. + private readonly string _sqlConnectionString = + $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared"; + + // Kept open for the factory lifetime so the shared-cache in-memory database is not dropped + // when per-scope connections are disposed between requests. + private SqliteConnection? _keepAliveConnection; protected override void ConfigureWebHost(IWebHostBuilder builder) { @@ -48,7 +54,19 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) services.Remove(migrationServiceDescriptor); } - services.AddSingleton(_ => CreateOpenSqliteConnection()); + // Open a keep-alive connection to prevent the shared-cache in-memory database from + // being dropped when per-scope connections are disposed between requests. + _keepAliveConnection = new SqliteConnection(_sqlConnectionString); + _keepAliveConnection.Open(); + + // Register as scoped so each request scope gets its own SqliteConnection, + // preventing "database is locked" errors under concurrent requests. + services.AddScoped(_ => + { + SqliteConnection conn = new(_sqlConnectionString); + conn.Open(); + return conn; + }); services.AddDbContext((serviceProvider, options) => { @@ -84,11 +102,13 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); } - private static SqliteConnection CreateOpenSqliteConnection() + protected override void Dispose(bool disposing) { - SqliteConnection connection = new(SqlConnectionString); - connection.Open(); - return connection; + if (disposing) + { + _keepAliveConnection?.Dispose(); + } + base.Dispose(disposing); } private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider) : IHostedService From dff58d72d126cc0b7b265fd541c2a3d73e41f5f7 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 20:54:48 -0700 Subject: [PATCH 10/17] fix(tests): make WebApplicationFactory service overrides idempotent Remove prior matching registrations for DbContext options, DbConnection, DatabaseMigrationService, and EnsureCreatedHostedService before re-adding test overrides. Also keep the SQLite keep-alive connection creation guarded with ??= across repeated ConfigureServices invocations. This avoids duplicate EnsureCreated hosted services racing each other and failing CI with SQLite Error 1: table AspNetRoles already exists. --- .../WebApplicationFactory.cs | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 9001f487..08776f47 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -26,38 +26,42 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) { builder.ConfigureServices(services => { - ServiceDescriptor? dbContextDescriptor = services.SingleOrDefault( - d => d.ServiceType == - typeof(IDbContextOptionsConfiguration)); - - if (dbContextDescriptor != null) + for (int i = services.Count - 1; i >= 0; i--) { - services.Remove(dbContextDescriptor); + if (services[i].ServiceType == + typeof(IDbContextOptionsConfiguration)) + { + services.RemoveAt(i); + } } - ServiceDescriptor? dbConnectionDescriptor = - services.SingleOrDefault( - d => d.ServiceType == - typeof(DbConnection)); - - if (dbConnectionDescriptor != null) + for (int i = services.Count - 1; i >= 0; i--) { - services.Remove(dbConnectionDescriptor); + if (services[i].ServiceType == typeof(DbConnection)) + { + services.RemoveAt(i); + } } // Remove DatabaseMigrationService: it calls MigrateAsync which conflicts // with EnsureCreated() used below for the in-memory SQLite test database. - ServiceDescriptor? migrationServiceDescriptor = services.SingleOrDefault( - d => d.ImplementationType == typeof(DatabaseMigrationService)); - if (migrationServiceDescriptor != null) + for (int i = services.Count - 1; i >= 0; i--) { - services.Remove(migrationServiceDescriptor); + ServiceDescriptor descriptor = services[i]; + if (descriptor.ServiceType == typeof(IHostedService) && + descriptor.ImplementationType == typeof(DatabaseMigrationService)) + { + services.RemoveAt(i); + } } // Open a keep-alive connection to prevent the shared-cache in-memory database from // being dropped when per-scope connections are disposed between requests. - _keepAliveConnection = new SqliteConnection(_sqlConnectionString); - _keepAliveConnection.Open(); + _keepAliveConnection ??= new SqliteConnection(_sqlConnectionString); + if (_keepAliveConnection.State != System.Data.ConnectionState.Open) + { + _keepAliveConnection.Open(); + } // Register as scoped so each request scope gets its own SqliteConnection, // preventing "database is locked" errors under concurrent requests. @@ -75,6 +79,16 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); // Ensure schema exists before any hosted service that reads from the database. + for (int i = services.Count - 1; i >= 0; i--) + { + ServiceDescriptor descriptor = services[i]; + if (descriptor.ServiceType == typeof(IHostedService) && + descriptor.ImplementationType == typeof(EnsureCreatedHostedService)) + { + services.RemoveAt(i); + } + } + ServiceDescriptor ensureCreatedDescriptor = ServiceDescriptor.Singleton(); int firstHostedServiceIndex = -1; From c4445d2cf2920b1d92309232a92d1a66ba7dfa73 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 21:04:12 -0700 Subject: [PATCH 11/17] fix(tests): serialize SQLite EnsureCreated and disable auto redirect in functional tests Add a static SemaphoreSlim gate around EnsureCreatedHostedService.StartAsync so parallel host startup cannot race while creating schema in shared in-memory SQLite. Also use CreateClientWithoutAutoRedirect in FunctionalTests so redirect handling stays explicit in GetFollowingGetRedirectsAsync. --- EssentialCSharp.Web.Tests/FunctionalTests.cs | 6 +++--- .../WebApplicationFactory.cs | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/EssentialCSharp.Web.Tests/FunctionalTests.cs b/EssentialCSharp.Web.Tests/FunctionalTests.cs index fa72240a..14f129e4 100644 --- a/EssentialCSharp.Web.Tests/FunctionalTests.cs +++ b/EssentialCSharp.Web.Tests/FunctionalTests.cs @@ -13,7 +13,7 @@ public class FunctionalTests : IntegrationTestBase [Arguments("/alive")] public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl) { - using HttpClient client = Factory.CreateClient(); + using HttpClient client = CreateClientWithoutAutoRedirect(); using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -29,7 +29,7 @@ public async Task WhenTheApplicationStarts_ItCanLoadLoadPages(string relativeUrl [Arguments("/about?someOtherParam=value")] public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) { - using HttpClient client = Factory.CreateClient(); + using HttpClient client = CreateClientWithoutAutoRedirect(); using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, relativeUrl); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.OK); @@ -45,7 +45,7 @@ public async Task WhenPagesAreAccessed_TheyReturnHtml(string relativeUrl) [Test] public async Task WhenTheApplicationStarts_NonExistingPage_GivesCorrectStatusCode() { - using HttpClient client = Factory.CreateClient(); + using HttpClient client = CreateClientWithoutAutoRedirect(); using HttpResponseMessage response = await GetFollowingGetRedirectsAsync(client, "/non-existing-page1234"); await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.NotFound); diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 08776f47..e63e13a8 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -9,11 +9,13 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; +using System.Threading; namespace EssentialCSharp.Web.Tests; public sealed class WebApplicationFactory : TestWebApplicationFactory { + private static readonly SemaphoreSlim SchemaInitializationGate = new(1, 1); // One GUID per factory instance → each factory gets its own isolated in-memory database. private readonly string _sqlConnectionString = $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared"; @@ -129,10 +131,18 @@ private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider { public async Task StartAsync(CancellationToken cancellationToken) { - using IServiceScope scope = serviceProvider.CreateScope(); - EssentialCSharpWebContext dbContext = - scope.ServiceProvider.GetRequiredService(); - await dbContext.Database.EnsureCreatedAsync(cancellationToken); + await SchemaInitializationGate.WaitAsync(cancellationToken); + try + { + using IServiceScope scope = serviceProvider.CreateScope(); + EssentialCSharpWebContext dbContext = + scope.ServiceProvider.GetRequiredService(); + await dbContext.Database.EnsureCreatedAsync(cancellationToken); + } + finally + { + SchemaInitializationGate.Release(); + } } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; From 153572e198f639930acd7cb96dd816125fa6bc50 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 21:06:39 -0700 Subject: [PATCH 12/17] fix(tests): harden factory override checks and keep redirect handling explicit Use explicit single-or-none descriptor removal (throws if duplicates are present) for DbContext options, DbConnection, DatabaseMigrationService, and EnsureCreatedHostedService. Keep schema creation serialized via semaphore. Update FunctionalTests to use CreateClientWithoutAutoRedirect so manual redirect handling remains intentional. --- .../WebApplicationFactory.cs | 81 +++++++++++-------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index e63e13a8..8c717b28 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -16,7 +16,8 @@ namespace EssentialCSharp.Web.Tests; public sealed class WebApplicationFactory : TestWebApplicationFactory { private static readonly SemaphoreSlim SchemaInitializationGate = new(1, 1); - // One GUID per factory instance → each factory gets its own isolated in-memory database. + // One GUID per factory instance (field, not computed property) ensures each factory + // gets its own isolated in-memory database and keeps a stable connection string. private readonly string _sqlConnectionString = $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared"; @@ -28,34 +29,24 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) { builder.ConfigureServices(services => { - for (int i = services.Count - 1; i >= 0; i--) - { - if (services[i].ServiceType == - typeof(IDbContextOptionsConfiguration)) - { - services.RemoveAt(i); - } - } + RemoveSingleOrNone( + services, + descriptor => descriptor.ServiceType == + typeof(IDbContextOptionsConfiguration), + "IDbContextOptionsConfiguration"); - for (int i = services.Count - 1; i >= 0; i--) - { - if (services[i].ServiceType == typeof(DbConnection)) - { - services.RemoveAt(i); - } - } + RemoveSingleOrNone( + services, + descriptor => descriptor.ServiceType == typeof(DbConnection), + nameof(DbConnection)); // Remove DatabaseMigrationService: it calls MigrateAsync which conflicts // with EnsureCreated() used below for the in-memory SQLite test database. - for (int i = services.Count - 1; i >= 0; i--) - { - ServiceDescriptor descriptor = services[i]; - if (descriptor.ServiceType == typeof(IHostedService) && - descriptor.ImplementationType == typeof(DatabaseMigrationService)) - { - services.RemoveAt(i); - } - } + RemoveSingleOrNone( + services, + descriptor => descriptor.ServiceType == typeof(IHostedService) && + descriptor.ImplementationType == typeof(DatabaseMigrationService), + nameof(DatabaseMigrationService)); // Open a keep-alive connection to prevent the shared-cache in-memory database from // being dropped when per-scope connections are disposed between requests. @@ -81,15 +72,11 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) }); // Ensure schema exists before any hosted service that reads from the database. - for (int i = services.Count - 1; i >= 0; i--) - { - ServiceDescriptor descriptor = services[i]; - if (descriptor.ServiceType == typeof(IHostedService) && - descriptor.ImplementationType == typeof(EnsureCreatedHostedService)) - { - services.RemoveAt(i); - } - } + RemoveSingleOrNone( + services, + descriptor => descriptor.ServiceType == typeof(IHostedService) && + descriptor.ImplementationType == typeof(EnsureCreatedHostedService), + nameof(EnsureCreatedHostedService)); ServiceDescriptor ensureCreatedDescriptor = ServiceDescriptor.Singleton(); @@ -127,6 +114,32 @@ protected override void Dispose(bool disposing) base.Dispose(disposing); } + private static void RemoveSingleOrNone( + IServiceCollection services, + Func predicate, + string descriptorName) + { + List matchIndexes = []; + for (int i = 0; i < services.Count; i++) + { + if (predicate(services[i])) + { + matchIndexes.Add(i); + } + } + + if (matchIndexes.Count > 1) + { + throw new InvalidOperationException( + $"Expected at most one '{descriptorName}' registration but found {matchIndexes.Count}."); + } + + if (matchIndexes.Count == 1) + { + services.RemoveAt(matchIndexes[0]); + } + } + private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider) : IHostedService { public async Task StartAsync(CancellationToken cancellationToken) From 761b9d5ea4cd89bf81ff9f456e2ae6d040d6f25b Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 21:36:26 -0700 Subject: [PATCH 13/17] refactor(tests): address latest PR feedback on tracing and helper contracts - Use traced client creation paths in IntegrationTestBase and McpTestHelper - Make InServiceScope helper methods protected - Clarify GET-only redirect helper intent - Remove unnecessary explicit System.Threading using - Use pooled SQLite connection string and clarify scoped DbConnection ownership - Insert EnsureCreatedHostedService at index 0 for deterministic startup ordering - Remove global schema semaphore after hardening descriptor replacement checks --- .../IntegrationTestBase.cs | 14 ++++--- EssentialCSharp.Web.Tests/McpTestHelper.cs | 2 +- .../WebApplicationFactory.cs | 40 ++++--------------- 3 files changed, 18 insertions(+), 38 deletions(-) diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 1f14ba99..8f344ba9 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -13,8 +13,12 @@ public abstract class IntegrationTestBase : WebApplicationTest protected HttpClient CreateClientWithoutAutoRedirect() => - Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + Factory.CreateClient(); + /// + /// Executes an initial GET and follows redirect responses with subsequent GET requests. + /// This helper is intentionally GET-only. + /// protected static async Task GetFollowingGetRedirectsAsync( HttpClient client, string relativeUrl, @@ -53,25 +57,25 @@ private static bool IsRedirectStatusCode(HttpStatusCode statusCode) => statusCode == HttpStatusCode.TemporaryRedirect || statusCode == HttpStatusCode.PermanentRedirect; - public T InServiceScope(Func action) + protected T InServiceScope(Func action) { using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); return action(scope.ServiceProvider); } - public void InServiceScope(Action action) + protected void InServiceScope(Action action) { using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); action(scope.ServiceProvider); } - public async Task InServiceScopeAsync(Func> action) + protected async Task InServiceScopeAsync(Func> action) { using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); return await action(scope.ServiceProvider); } - public async Task InServiceScopeAsync(Func action) + protected async Task InServiceScopeAsync(Func action) { using IServiceScope scope = Factory.Services.GetRequiredService().CreateScope(); await action(scope.ServiceProvider); diff --git a/EssentialCSharp.Web.Tests/McpTestHelper.cs b/EssentialCSharp.Web.Tests/McpTestHelper.cs index e564da4f..b5e068b7 100644 --- a/EssentialCSharp.Web.Tests/McpTestHelper.cs +++ b/EssentialCSharp.Web.Tests/McpTestHelper.cs @@ -16,7 +16,7 @@ namespace EssentialCSharp.Web.Tests; internal static class McpTestHelper { public static HttpClient CreateClient(TracedWebApplicationFactory factory) => - factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); + factory.CreateClient(); public static HttpRequestMessage CreateInitializeRequest(string path = "/mcp") { diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 8c717b28..cf8a2692 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -9,17 +9,15 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; -using System.Threading; namespace EssentialCSharp.Web.Tests; public sealed class WebApplicationFactory : TestWebApplicationFactory { - private static readonly SemaphoreSlim SchemaInitializationGate = new(1, 1); // One GUID per factory instance (field, not computed property) ensures each factory // gets its own isolated in-memory database and keeps a stable connection string. private readonly string _sqlConnectionString = - $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared"; + $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared;Pooling=True"; // Kept open for the factory lifetime so the shared-cache in-memory database is not dropped // when per-scope connections are disposed between requests. @@ -65,6 +63,8 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) return conn; }); + // The scoped DI container owns this DbConnection instance and disposes it at + // scope end; EF Core treats it as externally owned when passed via UseSqlite. services.AddDbContext((serviceProvider, options) => { DbConnection dbConnection = serviceProvider.GetRequiredService(); @@ -80,23 +80,7 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) ServiceDescriptor ensureCreatedDescriptor = ServiceDescriptor.Singleton(); - int firstHostedServiceIndex = -1; - for (int i = 0; i < services.Count; i++) - { - if (services[i].ServiceType == typeof(IHostedService)) - { - firstHostedServiceIndex = i; - break; - } - } - if (firstHostedServiceIndex >= 0) - { - services.Insert(firstHostedServiceIndex, ensureCreatedDescriptor); - } - else - { - services.Add(ensureCreatedDescriptor); - } + services.Insert(0, ensureCreatedDescriptor); // Replace IListingSourceCodeService with one backed by TestData services.RemoveAll(); @@ -144,18 +128,10 @@ private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider { public async Task StartAsync(CancellationToken cancellationToken) { - await SchemaInitializationGate.WaitAsync(cancellationToken); - try - { - using IServiceScope scope = serviceProvider.CreateScope(); - EssentialCSharpWebContext dbContext = - scope.ServiceProvider.GetRequiredService(); - await dbContext.Database.EnsureCreatedAsync(cancellationToken); - } - finally - { - SchemaInitializationGate.Release(); - } + using IServiceScope scope = serviceProvider.CreateScope(); + EssentialCSharpWebContext dbContext = + scope.ServiceProvider.GetRequiredService(); + await dbContext.Database.EnsureCreatedAsync(cancellationToken); } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; From 4cc987d06c66884121d560e91757c611ba0e8c1f Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 21:42:02 -0700 Subject: [PATCH 14/17] fix(tests): serialize schema creation per test factory Use a per-factory SemaphoreSlim for EnsureCreatedHostedService so concurrent host startup for the same WebApplicationFactory cannot race while creating the in-memory SQLite schema. Remove Pooling=True from the in-memory SQLite connection string to keep database lifetime governed by the explicit keep-alive connection. --- .../WebApplicationFactory.cs | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index cf8a2692..7fd21ddf 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -17,7 +17,9 @@ public sealed class WebApplicationFactory : TestWebApplicationFactory // One GUID per factory instance (field, not computed property) ensures each factory // gets its own isolated in-memory database and keeps a stable connection string. private readonly string _sqlConnectionString = - $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared;Pooling=True"; + $"DataSource=file:{Guid.NewGuid():N}?mode=memory&cache=shared"; + + private readonly SemaphoreSlim _schemaInitializationGate = new(1, 1); // Kept open for the factory lifetime so the shared-cache in-memory database is not dropped // when per-scope connections are disposed between requests. @@ -79,7 +81,10 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) nameof(EnsureCreatedHostedService)); ServiceDescriptor ensureCreatedDescriptor = - ServiceDescriptor.Singleton(); + ServiceDescriptor.Singleton( + serviceProvider => new EnsureCreatedHostedService( + serviceProvider, + _schemaInitializationGate)); services.Insert(0, ensureCreatedDescriptor); // Replace IListingSourceCodeService with one backed by TestData @@ -124,14 +129,24 @@ private static void RemoveSingleOrNone( } } - private sealed class EnsureCreatedHostedService(IServiceProvider serviceProvider) : IHostedService + private sealed class EnsureCreatedHostedService( + IServiceProvider serviceProvider, + SemaphoreSlim schemaInitializationGate) : IHostedService { public async Task StartAsync(CancellationToken cancellationToken) { - using IServiceScope scope = serviceProvider.CreateScope(); - EssentialCSharpWebContext dbContext = - scope.ServiceProvider.GetRequiredService(); - await dbContext.Database.EnsureCreatedAsync(cancellationToken); + await schemaInitializationGate.WaitAsync(cancellationToken); + try + { + using IServiceScope scope = serviceProvider.CreateScope(); + EssentialCSharpWebContext dbContext = + scope.ServiceProvider.GetRequiredService(); + await dbContext.Database.EnsureCreatedAsync(cancellationToken); + } + finally + { + schemaInitializationGate.Release(); + } } public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; From 794da8163d67712d38b3ec3df5c808ed7832d036 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 22:32:33 -0700 Subject: [PATCH 15/17] fix(tests): restore non-redirecting clients and harden keep-alive init - Restore AllowAutoRedirect=false clients via Factory.Inner.CreateClient(...) in IntegrationTestBase and McpTestHelper - Initialize keep-alive SQLite connection in WebApplicationFactory constructor - Keep per-factory schema initialization gate to prevent EnsureCreated races in CI --- .../IntegrationTestBase.cs | 2 +- EssentialCSharp.Web.Tests/McpTestHelper.cs | 2 +- .../WebApplicationFactory.cs | 17 +++++++++-------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index 8f344ba9..cf2fd191 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -13,7 +13,7 @@ public abstract class IntegrationTestBase : WebApplicationTest protected HttpClient CreateClientWithoutAutoRedirect() => - Factory.CreateClient(); + Factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); /// /// Executes an initial GET and follows redirect responses with subsequent GET requests. diff --git a/EssentialCSharp.Web.Tests/McpTestHelper.cs b/EssentialCSharp.Web.Tests/McpTestHelper.cs index b5e068b7..e564da4f 100644 --- a/EssentialCSharp.Web.Tests/McpTestHelper.cs +++ b/EssentialCSharp.Web.Tests/McpTestHelper.cs @@ -16,7 +16,7 @@ namespace EssentialCSharp.Web.Tests; internal static class McpTestHelper { public static HttpClient CreateClient(TracedWebApplicationFactory factory) => - factory.CreateClient(); + factory.Inner.CreateClient(new WebApplicationFactoryClientOptions { AllowAutoRedirect = false }); public static HttpRequestMessage CreateInitializeRequest(string path = "/mcp") { diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 7fd21ddf..250e8511 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -23,7 +23,13 @@ public sealed class WebApplicationFactory : TestWebApplicationFactory // Kept open for the factory lifetime so the shared-cache in-memory database is not dropped // when per-scope connections are disposed between requests. - private SqliteConnection? _keepAliveConnection; + private readonly SqliteConnection _keepAliveConnection; + + public WebApplicationFactory() + { + _keepAliveConnection = new SqliteConnection(_sqlConnectionString); + _keepAliveConnection.Open(); + } protected override void ConfigureWebHost(IWebHostBuilder builder) { @@ -50,12 +56,6 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) // Open a keep-alive connection to prevent the shared-cache in-memory database from // being dropped when per-scope connections are disposed between requests. - _keepAliveConnection ??= new SqliteConnection(_sqlConnectionString); - if (_keepAliveConnection.State != System.Data.ConnectionState.Open) - { - _keepAliveConnection.Open(); - } - // Register as scoped so each request scope gets its own SqliteConnection, // preventing "database is locked" errors under concurrent requests. services.AddScoped(_ => @@ -98,7 +98,8 @@ protected override void Dispose(bool disposing) { if (disposing) { - _keepAliveConnection?.Dispose(); + _keepAliveConnection.Dispose(); + _schemaInitializationGate.Dispose(); } base.Dispose(disposing); } From 50265198df7b9306302e17ce4f6ae815ca8f31ce Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sat, 16 May 2026 23:18:20 -0700 Subject: [PATCH 16/17] fix(tests): dispose response before redirect-limit exception Dispose the last HttpResponseMessage in GetFollowingGetRedirectsAsync before throwing when max redirects is exceeded. --- EssentialCSharp.Web.Tests/IntegrationTestBase.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs index cf2fd191..a526b486 100644 --- a/EssentialCSharp.Web.Tests/IntegrationTestBase.cs +++ b/EssentialCSharp.Web.Tests/IntegrationTestBase.cs @@ -43,6 +43,7 @@ protected static async Task GetFollowingGetRedirectsAsync( if (IsRedirectStatusCode(response.StatusCode)) { + response.Dispose(); throw new InvalidOperationException( $"Exceeded redirect limit ({maxRedirects}) for '{relativeUrl}'. Last status: {(int)response.StatusCode} {response.StatusCode}."); } From 25cb216aae5aec6538c61d6e36a4ce98b72ef16e Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Sun, 17 May 2026 00:43:16 -0700 Subject: [PATCH 17/17] chore(tests): clarify hosted-service insertion and harden connection open Clarify that EnsureCreatedHostedService is prepended via Insert(0), and guard per-scope SqliteConnection.Open() with dispose-on-failure semantics. --- .../WebApplicationFactory.cs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs index 250e8511..cf7a978e 100644 --- a/EssentialCSharp.Web.Tests/WebApplicationFactory.cs +++ b/EssentialCSharp.Web.Tests/WebApplicationFactory.cs @@ -54,14 +54,20 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) descriptor.ImplementationType == typeof(DatabaseMigrationService), nameof(DatabaseMigrationService)); - // Open a keep-alive connection to prevent the shared-cache in-memory database from - // being dropped when per-scope connections are disposed between requests. - // Register as scoped so each request scope gets its own SqliteConnection, - // preventing "database is locked" errors under concurrent requests. + // Keep per-scope connections so each request/test scope uses a fresh DbConnection, + // which avoids locking issues from sharing one SqliteConnection instance. services.AddScoped(_ => { SqliteConnection conn = new(_sqlConnectionString); - conn.Open(); + try + { + conn.Open(); + } + catch + { + conn.Dispose(); + throw; + } return conn; }); @@ -73,7 +79,8 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) options.UseSqlite(dbConnection); }); - // Ensure schema exists before any hosted service that reads from the database. + // Ensure schema exists before hosted services by prepending this registration + // at index 0 of the service collection. RemoveSingleOrNone( services, descriptor => descriptor.ServiceType == typeof(IHostedService) &&