From 61658282c76abf0bd25e208911aae966f6538d78 Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Sun, 13 Sep 2026 13:00:11 +0200 Subject: [PATCH 1/2] test(registration): age the reaper's ghost by the database clock, not the test's The reaper ages a candidate by its stream's `created` stamp, which Marten leaves to Postgres (`now()`), while the cutoff comes from the process clock. With `olderThanDays: 0` the test compared the two at zero margin, so a Docker VM clock a second ahead of Windows made it miss the ghost in streaks. Backdate the ghost's stream in `mt_streams` by ten days, run the reaper with the real seven-day window, and add a just-created ghost that must survive it, so the test now also proves the age window is enforced. Co-Authored-By: Claude Fable 5.1 --- .../RegistrationPipelineTests.cs | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/dotnet/Modgud.Api.Tests/Authorization/RegistrationPipelineTests.cs b/src/dotnet/Modgud.Api.Tests/Authorization/RegistrationPipelineTests.cs index 13b0796c..fe618a6c 100644 --- a/src/dotnet/Modgud.Api.Tests/Authorization/RegistrationPipelineTests.cs +++ b/src/dotnet/Modgud.Api.Tests/Authorization/RegistrationPipelineTests.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; +using Npgsql; using Modgud.Api.Features.Admin.Jobs; using Modgud.Api.Tests.Infrastructure; using Modgud.Application.DTOs.OAuth; @@ -236,9 +237,11 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() { var ct = TestContext.Current.CancellationToken; const string ghostEmail = "rp-ghost@example.test"; + const string youngEmail = "rp-young-ghost@example.test"; const string keepEmail = "rp-keep-password@example.test"; + const int olderThanDays = 7; - Guid ghostId, keepId; + Guid ghostId, youngId, keepId; using (var scope = CreateTenantScope()) { var userManager = scope.ServiceProvider.GetRequiredService>(); @@ -254,6 +257,23 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() CreatedAt = DateTimeOffset.UtcNow.AddDays(-10), ExpiresAt = DateTimeOffset.UtcNow.AddDays(-10), }); await session.SaveChangesAsync(ct); + // The reaper ages a candidate by its STREAM's creation time, and Postgres stamps + // that with its own now() — not this process's clock. Backdate it in the database + // so the ghost is old by any clock; the assertion must never ride on the DB host + // and the test host agreeing to the second (a Docker VM clock running ahead of + // Windows made a zero-day window miss the ghost in streaks). + await BackdateStreamAsync(session, ghost.Id, TimeSpan.FromDays(10), ct); + + // Same signature, but created just now → inside the window, must survive. + var young = new ApplicationUser(youngEmail, youngEmail) { IsActive = true }; + Assert.True((await userManager.CreateAsync(young)).Succeeded); + youngId = young.Id; + session.Store(new EmailOtpChallenge + { + Id = young.Id, CodeHash = "dead", Email = youngEmail, + CreatedAt = DateTimeOffset.UtcNow, ExpiresAt = DateTimeOffset.UtcNow.AddMinutes(10), + }); + await session.SaveChangesAsync(ct); // Unconfirmed but WITH a password → outside the signature, must survive. var keep = new ApplicationUser("rp-keep-password", keepEmail) { IsActive = true }; @@ -264,7 +284,7 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() using (var scope = CreateTenantScope()) { var job = ActivatorUtilities.CreateInstance(scope.ServiceProvider); - var dry = await job.RunAsync(dryRun: true, olderThanDays: 0, ct); + var dry = await job.RunAsync(dryRun: true, olderThanDays, ct); Assert.Equal(1, dry.Matched); Assert.Equal(0, dry.Erased); } @@ -273,7 +293,7 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() using (var scope = CreateTenantScope()) { var job = ActivatorUtilities.CreateInstance(scope.ServiceProvider); - var run = await job.RunAsync(dryRun: false, olderThanDays: 0, ct); + var run = await job.RunAsync(dryRun: false, olderThanDays, ct); Assert.Equal((1, 1), run); } @@ -283,6 +303,9 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() var session = scope.ServiceProvider.GetRequiredService(); var erased = await session.LoadAsync(ghostId, ct); Assert.True(erased is null || erased.IsDeleted); + var young = await session.LoadAsync(youngId, ct); + Assert.NotNull(young); + Assert.False(young!.IsDeleted); var kept = await session.LoadAsync(keepId, ct); Assert.NotNull(kept); Assert.False(kept!.IsDeleted); @@ -290,6 +313,24 @@ public async Task Reaper_erases_only_legacy_ghosts_and_only_when_not_dry_run() } } + /// + /// Moves a stream's created stamp into the past. Marten leaves that column to the + /// database default (now()) and offers no API to set it, so age-based tests have to + /// reach into mt_streams directly. + /// + private static async Task BackdateStreamAsync(IDocumentSession session, Guid streamId, TimeSpan by, CancellationToken ct) + { + var schema = session.DocumentStore.Options.Events.DatabaseSchemaName; + // CA2100: the schema name is Marten's own configuration, not input; the values are bound. +#pragma warning disable CA2100 + await using var cmd = new NpgsqlCommand($"update {schema}.mt_streams set created = created - @by where id = @id"); +#pragma warning restore CA2100 + cmd.Parameters.AddWithValue("by", by); + cmd.Parameters.AddWithValue("id", streamId); + var rows = await session.ExecuteAsync(cmd, ct); + Assert.Equal(1, rows); + } + // ── Helpers ────────────────────────────────────────────────────────────── private async Task SetUpJitAppAsync(string slug, string host) From c37347fd160f835d63ef7859c850fd5e9189492f Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Sun, 13 Sep 2026 13:34:16 +0200 Subject: [PATCH 2/2] test(infra): give the Testcontainers fixtures a 60 s connect timeout Booting a host per test opens fresh connections through Docker Desktop's published-port proxy while other containers may be starting or stopping. A TCP connect that stalls past Npgsql's 15 s default surfaced once in ten full ColdStart runs as "Timeout during connection attempt" during ApplyAllConfiguredChangesToDatabaseAsync and failed an unrelated test. Realm databases derive their connection strings from the fixture's, so the setting reaches every database the suites touch. Co-Authored-By: Claude Fable 5.1 --- .../Infrastructure/ColdStartFixture.cs | 2 +- .../Infrastructure/SharedPostgresFixture.cs | 2 +- .../Infrastructure/TestPostgres.cs | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 src/dotnet/Modgud.Api.Tests/Infrastructure/TestPostgres.cs diff --git a/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartFixture.cs b/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartFixture.cs index d42c594a..f73492bd 100644 --- a/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartFixture.cs +++ b/src/dotnet/Modgud.Api.Tests/Infrastructure/ColdStartFixture.cs @@ -115,7 +115,7 @@ private static TestConfigurationContext BuildContext(string connectionString) => CertPath = null, // Disable certificate loading DbSettings = { - ConnectionString = connectionString + ConnectionString = TestPostgres.WithGenerousConnectTimeout(connectionString) } }), rule.For().FromStatic(_ => new AppSettings { AuthenticationMinimumLevel = 0 }), diff --git a/src/dotnet/Modgud.Api.Tests/Infrastructure/SharedPostgresFixture.cs b/src/dotnet/Modgud.Api.Tests/Infrastructure/SharedPostgresFixture.cs index 73c16e17..dbdb26d0 100644 --- a/src/dotnet/Modgud.Api.Tests/Infrastructure/SharedPostgresFixture.cs +++ b/src/dotnet/Modgud.Api.Tests/Infrastructure/SharedPostgresFixture.cs @@ -24,7 +24,7 @@ public class SharedPostgresFixture : IAsyncLifetime .WithCommand("-c", "max_connections=500") .Build(); - public string ConnectionString => Container.GetConnectionString(); + public string ConnectionString => TestPostgres.WithGenerousConnectTimeout(Container.GetConnectionString()); /// /// Test configuration context - created after container starts. diff --git a/src/dotnet/Modgud.Api.Tests/Infrastructure/TestPostgres.cs b/src/dotnet/Modgud.Api.Tests/Infrastructure/TestPostgres.cs new file mode 100644 index 00000000..6a47e2f3 --- /dev/null +++ b/src/dotnet/Modgud.Api.Tests/Infrastructure/TestPostgres.cs @@ -0,0 +1,19 @@ +using Npgsql; + +namespace Modgud.Api.Tests.Infrastructure; + +/// Connection-string adjustments shared by the Testcontainers fixtures. +public static class TestPostgres +{ + /// + /// Raises Npgsql's connect timeout from its 15 s default. The suites boot a host per + /// test, and each boot opens fresh connections through Docker Desktop's published-port + /// proxy while other containers may be starting or stopping. A TCP connect that stalls + /// past 15 s surfaces as "Timeout during connection attempt" at + /// ApplyAllConfiguredChangesToDatabaseAsync and fails an unrelated test once in + /// a long run. Realm databases derive their connection strings from this one, so the + /// setting reaches them too. The value also bounds the wait for a pooled connection. + /// + public static string WithGenerousConnectTimeout(string connectionString) => + new NpgsqlConnectionStringBuilder(connectionString) { Timeout = 60 }.ConnectionString; +}