tests: migrate integration tests to TUnit.AspNetCore WebApplicationTest#1100
Open
BenjaminMichaelis wants to merge 5 commits into
Open
tests: migrate integration tests to TUnit.AspNetCore WebApplicationTest#1100BenjaminMichaelis wants to merge 5 commits into
BenjaminMichaelis wants to merge 5 commits into
Conversation
- Add TUnit.AspNetCore 1.40.5 package reference - Create IntegrationTestBase : WebApplicationTest<WebApplicationFactory, Program> for per-test factory isolation, InServiceScope helpers, and trace correlation - Rewrite WebApplicationFactory to extend TestWebApplicationFactory<Program> (removes IAsyncInitializer, moves to ConcurrentBag for multi-connection tracking) - Migrate all test classes to inherit from IntegrationTestBase: - Remove [NotInParallel] and [ClassDataSource<WebApplicationFactory>] attributes - Collapse 6 McpRateLimitingTests classes into 1 (fresh DI per test = fresh rate limiter) - Update McpTestHelper to accept TracedWebApplicationFactory<Program> - Use Factory.Inner.CreateClient(options) where AllowAutoRedirect=false is needed - Remove TUnit.Core.Interfaces import (IAsyncInitializer no longer used)
… 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.
…pe 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<T> 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.
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the integration test project from a hand-rolled WebApplicationFactory<Program> + IAsyncInitializer setup to TUnit.AspNetCore's first-class WebApplicationTest<TFactory, TEntryPoint> pattern, giving per-test factory isolation, trace correlation, and removing the need for [NotInParallel] and [ClassDataSource] attributes.
Changes:
- Adds
TUnit.AspNetCore1.40.5; subclassesTestWebApplicationFactory<Program>and tracks SQLite connections in aConcurrentBagwithInterlockeddisposal guard. - Introduces
IntegrationTestBaseprovidingInServiceScope[Async]helpers and aCreateRedirectFollowingClient()escape hatch for redirect-following clients. - Refactors all integration test classes to inherit
IntegrationTestBase; merges six MCP rate-limiting classes into one (each method now gets a fresh factory/limiter).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Directory.Packages.props | Add TUnit.AspNetCore 1.40.5 package version. |
| EssentialCSharp.Web.Tests/EssentialCSharp.Web.Tests.csproj | Reference new TUnit.AspNetCore package. |
| EssentialCSharp.Web.Tests/WebApplicationFactory.cs | Switch base type, track connections in ConcurrentBag, add disposal guard. |
| EssentialCSharp.Web.Tests/IntegrationTestBase.cs | New base class with service-scope helpers and redirect-following client helper. |
| EssentialCSharp.Web.Tests/McpTestHelper.cs | Accept TracedWebApplicationFactory<Program>; drop AllowAutoRedirect=false. |
| EssentialCSharp.Web.Tests/McpRateLimitingTests.cs | Collapse six classes into one with six tests; remove [NotInParallel]. |
| EssentialCSharp.Web.Tests/McpTests.cs | Use new base; replace factory with Factory. |
| EssentialCSharp.Web.Tests/McpToolContractTests.cs | Use new base; replace factory with Factory. |
| EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs | Use new base; replace factory with Factory. |
| EssentialCSharp.Web.Tests/ContentRateLimitingTests.cs | Use new base; inline AllowAutoRedirect=false no longer needed (default). |
| EssentialCSharp.Web.Tests/FunctionalTests.cs | Use new base and CreateRedirectFollowingClient() to preserve redirect-following behavior. |
| EssentialCSharp.Web.Tests/ListingSourceCodeControllerTests.cs | Use new base; replace factory with Factory. |
| EssentialCSharp.Web.Tests/RouteConfigurationServiceTests.cs | Use new base; call InServiceScope on the base. |
| EssentialCSharp.Web.Tests/SitemapXmlHelpersTests.cs | Use new base; replace _Factory with Factory. |
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
EssentialCSharp.Web.Testsfrom a hand-rolledWebApplicationFactory<Program>+IAsyncInitializersetup toTUnit.AspNetCore's first-classWebApplicationTest<TFactory, TEntryPoint>pattern.All 130 tests pass.
Changes
New package
TUnit.AspNetCore1.40.5 toDirectory.Packages.propsand the test.csprojWebApplicationFactory.csWebApplicationFactory<Program>→TestWebApplicationFactory<Program>TestContext.Currentin request handlersTUnit0064analyzer warningIAsyncInitializer— TUnit manages factory lifecycleConcurrentBag<SqliteConnection>for thread-safe connection trackingInterlockeddisposal guard to prevent double-disposeIntegrationTestBase.cs(new)abstract class IntegrationTestBase : WebApplicationTest<WebApplicationFactory, Program>[ClassDataSource]+ constructor injection across all test classesInServiceScope<T>,InServiceScopeAsync<T>,CreateRedirectFollowingClient()McpTestHelper.csTracedWebApplicationFactory<Program>instead ofWebApplicationFactory<Program>CreateClientusesfactory.CreateClient()— restoresActivityPropagationHandlertrace correlation for all MCP testsMcpRateLimitingTests.cs[ClassDataSource]+[NotInParallel]classes → 1 class with 6 methodsIHost= fresh rate limiter) removes the need for[NotInParallel]All other test classes
McpTests,McpToolContractTests,McpApiTokenServiceTests,ContentRateLimitingTests,FunctionalTests,ListingSourceCodeControllerTests,RouteConfigurationServiceTests,SitemapXmlHelpersTests[ClassDataSource<WebApplicationFactory>],[NotInParallel], constructor injectionIntegrationTestBaseKey decisions
Factory.CreateClient()defaultAllowAutoRedirect=false+ full trace correlation viaActivityPropagationHandlerCreateRedirectFollowingClient()TracedWebApplicationFactoryhas noCreateClient(options)overload;Inneris the only redirect path. Named + documented as explicit trade-off[NotInParallel]IHost= isolated rate limiter state per testReviewer notes
ASP0000(BuildServiceProviderinConfigureWebHost) is pre-existing, not introduced hereTracedWebApplicationFactory<T>(theFactoryproperty) is TUnit's per-test wrapper;GlobalFactorywould expose custom factory properties — not needed here