test(org): isolate in-memory store per fixture

TestWebApplicationFactory instances shared the same in-memory database
because EF Core's UseInMemoryDatabase("InMemory") returns the same
backing store to every DbContext that asks for it under the same
connection string, in the same process. Whichever fixture started
first defined the state, and every subsequent fixture inherited it,
making tests silently order-dependent and flaky.

Fix:

- Yavsc.Tests.Shared/InMemoryDatabaseName: helper that suffixes the
  in-memory connection string with a per-fixture GUID.
- TestWebApplicationFactory: instance GUID + ConnectionStrings__
  YavscConnection set as an environment variable in the constructor
  and cleared in Dispose, so each factory gets its own backing store.
  Env var is needed because IdentityServer8.EntityFramework exposes
  ConfigureDbContext as Action<DbContextOptionsBuilder> with no
  service-provider access, so the connection string is captured at
  registration time. AddEnvironmentVariables is the last provider in
  the config pipeline and wins regardless.
- WebServerFixture: process-static GUID (WebHostFixture is a
  per-process singleton by design, so the test collection shares one
  store; the GUID still isolates from TestWebApplicationFactory).
- AddIdentityDBAndStores: read the connection string at DbContext
  construction time via the (sp, options) overload of AddDbContext,
  so test fixtures can override it via the host's IConfiguration.
  IdentityServer stores cannot do the same without subclassing the
  framework's DbContexts; the env var path is the documented escape
  hatch in HostingExtensions.AddIdentityServer.
- UsesInMemoryProvider: StartsWith instead of equality, so
  'InMemory-{guid}' is still recognised as an in-memory connection
  string.

Regression sentinel in
Controllers/TestWebApplicationFactoryIsolationTests: two factories
seed a marker client in the first, the second must not see it.

Suite: 45/45 over 3 stable runs, 13-15s each.
This commit is contained in:
Paul Schneider 2026-08-22 04:36:52 +01:00
commit 61b41f0c55
Signed by: notazof
GPG key ID: 1DD5D838E5343B06
5 changed files with 201 additions and 6 deletions

View file

@ -0,0 +1,68 @@
using IdentityServer8.EntityFramework.DbContexts;
using IdentityServer8.EntityFramework.Entities;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
namespace Yavsc.Org.Tests.Controllers;
/// <summary>
/// Regression sentinel: two <see cref="TestWebApplicationFactory"/>
/// instances must not see each other's clients.
///
/// EF Core's <c>UseInMemoryDatabase(name)</c> returns the same
/// backing store to every <c>DbContext</c> that asks for it under
/// the same name, in the same process. Before the per-fixture GUID
/// fix, both <see cref="TestWebApplicationFactory"/> and
/// <see cref="WebServerFixture"/> used the bare <c>"InMemory"</c>
/// connection string, so every fixture shared one store and tests
/// were silently order-dependent.
///
/// We assert against <see cref="ConfigurationDbContext"/> directly
/// rather than via <c>IClientStore</c>: the validating wrapper around
/// <c>IClientStore</c> raises events through <c>IEventService</c>,
/// which is not registered in the test host and crashes with a
/// <c>NullReferenceException</c> before it can return a result. Going
/// straight to the DbContext is the same code path the production
/// code uses, so it is the right surface to assert against.
/// </summary>
public class TestWebApplicationFactoryIsolationTests
{
[Fact]
public async Task Second_factory_does_not_see_clients_seeded_into_first()
{
var marker = $"marker-A-{Guid.NewGuid():N}";
// First factory: seed a distinctive client.
using (var first = new TestWebApplicationFactory())
{
await using var scope = first.Services.CreateAsyncScope();
var configDb = scope.ServiceProvider.GetRequiredService<ConfigurationDbContext>();
var firstCs = scope.ServiceProvider.GetRequiredService<IConfiguration>()
.GetConnectionString("YavscConnection");
Assert.StartsWith("InMemory-", firstCs);
configDb.Clients.Add(new Client { ClientId = marker, ClientName = "marker-A" });
await configDb.SaveChangesAsync(TestContext.Current.CancellationToken);
// Sanity: the first factory can see its own seed.
var seenByFirst = await configDb.Clients
.AsNoTracking()
.AnyAsync(c => c.ClientId == marker, TestContext.Current.CancellationToken);
Assert.True(seenByFirst);
}
// Second factory: must start from a clean slate. If the
// in-memory store leaked from the first factory, this
// assertion fails.
using var second = new TestWebApplicationFactory();
await using var secondScope = second.Services.CreateAsyncScope();
var secondCs = secondScope.ServiceProvider.GetRequiredService<IConfiguration>()
.GetConnectionString("YavscConnection");
Assert.StartsWith("InMemory-", secondCs);
var secondDb = secondScope.ServiceProvider.GetRequiredService<ConfigurationDbContext>();
var seenBySecond = await secondDb.Clients
.AsNoTracking()
.AnyAsync(c => c.ClientId == marker, TestContext.Current.CancellationToken);
Assert.False(seenBySecond);
}
}

View file

@ -21,9 +21,54 @@ namespace Yavsc.Org.Tests;
/// <see cref="TestUserMiddleware"/> so that <c>User.GetUserId()</c>
/// in user code sees a logged-in identity derived from the same
/// header.
///
/// Each instance gets its own in-memory database, identified by a
/// GUID generated in the constructor. The connection string
/// (<c>ConnectionStrings:YavscConnection</c>) is set as an
/// environment variable (<c>ConnectionStrings__YavscConnection</c>)
/// in the constructor and unset in <see cref="Dispose"/>, so the
/// production <c>AddIdentityDBAndStores</c> registers <c>DbContext</c>
/// instances against this fixture's own store. Without this, the
/// <c>"InMemory"</c> connection string from
/// <c>appsettings-org.Testing.json</c> would route every
/// <see cref="TestWebApplicationFactory"/> instance — and any
/// <see cref="WebServerFixture"/> running in the same process — to
/// the same backing store, leaking state between fixtures.
///
/// Env vars are used (rather than <c>ConfigureAppConfiguration</c> or
/// <c>UseSetting</c>) because <c>WebApplicationFactory</c> applies
/// those too late: <c>Program.Main</c> has already captured the
/// connection string in <c>AddIdentityDBAndStores</c> by the time
/// the test host's overrides take effect. Env vars are the last
/// provider added in <c>AddConfiguration</c> (see
/// <c>Yavsc.Server/Helpers/ConfigHelpers.cs</c>), so they win.
/// </summary>
public class TestWebApplicationFactory : WebApplicationFactory<Program>
{
private readonly string _fixtureId = Guid.NewGuid().ToString("N");
// ASP.NET Core's environment-variable configuration provider uses
// the key ConnectionStrings__YavscConnection (double underscore
// for the section separator). Set it before the host starts so
// the per-fixture connection string wins over
// appsettings-org.Testing.json. We do NOT touch the appsettings
// file; env vars take precedence in the configuration pipeline
// (see AddConfiguration in Yavsc.Server/Helpers/ConfigHelpers.cs,
// which adds AddEnvironmentVariables last).
private static readonly object _envLock = new();
private bool _envSet;
public TestWebApplicationFactory()
{
lock (_envLock)
{
Environment.SetEnvironmentVariable(
"ConnectionStrings__YavscConnection",
InMemoryDatabaseName.For(_fixtureId));
_envSet = true;
}
}
protected override void ConfigureWebHost(IWebHostBuilder builder)
{
// UseEnvironment("Testing") puts the host in a dedicated
@ -50,4 +95,18 @@ public class TestWebApplicationFactory : WebApplicationFactory<Program>
services.AddTransient<IStartupFilter, TestUserStartupFilter>();
});
}
protected override void Dispose(bool disposing)
{
if (disposing && _envSet)
{
lock (_envLock)
{
Environment.SetEnvironmentVariable(
"ConnectionStrings__YavscConnection", null);
_envSet = false;
}
}
base.Dispose(disposing);
}
}

View file

@ -42,6 +42,17 @@ public sealed class WebServerFixture : WebHostFixture
{
private static readonly int _httpsPort = GetAvailableLoopbackPort();
// One in-memory database name for the whole process: WebHostFixture
// is a per-process singleton (see _app, _isInitialized, _sharedServices
// in the base class), so every WebServerFixture instance shares the
// same backing store. That is intentional — the "Yavsc Server" test
// collection groups tests that should see the same seeded state, and
// re-initialising the store per fixture would just regress the
// order-dependence we are trying to eliminate. The GUID still matters
// because TestWebApplicationFactory and WebServerFixture must not
// collide in the in-memory store; see InMemoryDatabaseName.
private static readonly string _fixtureId = Guid.NewGuid().ToString("N");
protected override int HttpsPort => _httpsPort;
private static IConfiguration? _sharedConfiguration;
@ -80,7 +91,7 @@ public sealed class WebServerFixture : WebHostFixture
// that plus the in-memory overrides below.
builder.AddConfiguration(null).AddInMemoryCollection(new Dictionary<string, string?>
{
[$"ConnectionStrings:{Yavsc.Constants.YavscConnectionStringName}"] = "InMemory",
[$"ConnectionStrings:{Yavsc.Constants.YavscConnectionStringName}"] = InMemoryDatabaseName.For(_fixtureId),
// SMTP test config: UserName non-null so MailSender
// exercises the Authenticate branch — the
// RecordingSmtpClient captures it.

View file

@ -169,10 +169,20 @@ public static class HostingExtensions
public static IdentityBuilder AddIdentityDBAndStores(this WebApplicationBuilder builder)
{
IServiceCollection services = builder.Services;
var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName);
services.AddDbContext<ApplicationDbContext>(options =>
services.AddDbContext<ApplicationDbContext>((sp, options) =>
{
// Read the connection string at DbContext construction time
// rather than at AddDbContext registration time, so test
// fixtures (e.g. WebApplicationFactory<Program>) can
// override the value via the host's IConfiguration before
// any DbContext is built. Reading it eagerly at the top of
// this method would freeze whatever was in configuration
// when Program.Main ran — too early for the test host's
// ConfigureAppConfiguration / UseSetting hooks to apply.
var connectionString = sp.GetRequiredService<IConfiguration>()
.GetConnectionString(Constants.YavscConnectionStringName);
if (UsesInMemoryProvider(connectionString))
{
options.UseInMemoryDatabase(connectionString);
@ -317,9 +327,20 @@ public static class HostingExtensions
options.ClaimsIdentity.RoleClaimType = Constants.RoleClaimType;
});
var migrationsAssembly = typeof(Program).GetTypeInfo().Assembly.GetName().Name;
var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName);
string sqliteConnectionString = $"Data Source={Path.Combine(Path.GetTempPath(), "yavsc_test.db")}";
// The IdentityServer8.EntityFramework ConfigurationStoreOptions
// and OperationalStoreOptions expose ConfigureDbContext as an
// Action<DbContextOptionsBuilder> with no service-provider
// access, so the connection string has to be captured here at
// registration time. For the production runtime this is fine:
// the connection string does not change after startup. For
// tests, this is the one knob we cannot push into the per-fixture
// config pipeline; the TestWebApplicationFactory bridge instead
// sets ConnectionStrings__YavscConnection as an environment
// variable, which AddEnvironmentVariables picks up as the last
// configuration provider in AddConfiguration. See
// Yavsc.Server/Helpers/ConfigHelpers.cs.
var connectionString = builder.Configuration.GetConnectionString(Constants.YavscConnectionStringName);
var identityServerBuilder = builder.Services.AddIdentityServer(options =>
{
@ -600,7 +621,13 @@ public static class HostingExtensions
private static bool UsesInMemoryProvider(string connectionString)
{
return string.Equals(connectionString, InMemoryProviderName, StringComparison.OrdinalIgnoreCase);
// Test fixtures may suffix the connection string with a
// per-fixture GUID (see InMemoryDatabaseName in
// Yavsc.Tests.Shared) to keep their in-memory stores
// isolated. The base name "InMemory" is still what
// identifies an in-memory provider — anything starting
// with it is one.
return connectionString.StartsWith(InMemoryProviderName, StringComparison.OrdinalIgnoreCase);
}
private static Action<DbContext, bool> EnsureDefaultApplicationScopes()

View file

@ -0,0 +1,30 @@
namespace Yavsc.Tests.Shared;
/// <summary>
/// Helpers for the in-memory connection string used by test fixtures.
///
/// EF Core's <c>UseInMemoryDatabase(name)</c> returns the same backing
/// store to every <c>DbContext</c> that asks for it under the same
/// <paramref name="name"/>, in the same process. That means every
/// fixture that uses the bare <c>"InMemory"</c> connection string
/// shares the same in-memory database — which leaks state between
/// fixtures that are supposed to be independent, and silently makes
/// tests order-dependent.
///
/// The fix is to give each fixture its own suffix. <see cref="For"/>
/// returns a stable, fixture-scoped connection string. The fixture
/// stores the suffix in an instance field so successive calls within
/// the same fixture always resolve to the same database.
/// </summary>
public static class InMemoryDatabaseName
{
/// <summary>Base connection string for the in-memory provider,
/// as it appears in <c>appsettings-org.Testing.json</c>.</summary>
public const string Base = "InMemory";
/// <summary>Builds a per-fixture connection string. Two calls
/// with the same <paramref name="fixtureId"/> return the same
/// string; two calls with different ids return different
/// strings, isolating the underlying in-memory stores.</summary>
public static string For(string fixtureId) => $"{Base}-{fixtureId}";
}