diff --git a/CLAUDE.md b/CLAUDE.md index b86c10e..fd05e5f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -125,6 +125,50 @@ Critical path for MVP: Stages 1–5, 7–8, 10–11 xunit.v3 is built against Microsoft.Testing.Platform 1.x; anything 2.x makes every test run die at startup with a `TypeLoadException`. See the comment in `Directory.Packages.props`. +### Writing tests that survive being run in parallel processes + +The suite is a candidate for being run across several worker processes at once (Bobcat's supervisor +drives the MTP executable directly; measured **15.9 min → 5.7 min at four workers**). Each worker is +pointed at its own catalog through `POLECAT_TESTING_DATABASE`. Two rules follow, and both were +learned by watching tests break: + +**Never rewrite a connection string by text.** This was in three files and was silently wrong: + +```csharp +// WRONG — only rewrites while the catalog happens to be "master" +ConnectionSource.ConnectionString.Replace("Initial Catalog=master", $"Database={name}") +``` + +Point `POLECAT_TESTING_DATABASE` at anything else and the literal is absent, so the replace matches +nothing, the "other" database quietly resolves to the *current* one, and the test asserts against +itself — passing or failing for reasons unrelated to what it is testing. Use the helpers on +`ConnectionSource` instead: + +```csharp +ConnectionSource.ConnectionStringFor(name) // another database on the same server +ConnectionSource.MasterConnectionString // master, for DDL +ConnectionSource.DatabaseName // the catalog this process is using +``` + +**Name every database a test creates with `ConnectionSource.Scoped(...)`.** A database a test +creates is a *sibling* of the process's own database, not a child of it — so giving each worker its +own catalog does **not** isolate them. A hardcoded `"polecat_tenant_a"` is one database shared by +every worker on the box, and they will race to create and drop it: + +```csharp +// WRONG — every worker fights over the same database +private const string DbA = "polecat_tenant_a"; + +// RIGHT — "master_tenant_a" locally, "polecat_w3_tenant_a" under a parallel runner +private static readonly string DbA = ConnectionSource.Scoped("tenant_a"); +``` + +The same rule applies to anything else that lives at server scope rather than inside the catalog: +logins, linked servers, Agent jobs. + +Schema names do **not** need scoping — they live inside the catalog, so per-worker databases already +separate them. That is why `SchemaName = "doc_usage"` appearing in nine files is fine. + ## Engineering Principles - Mirror Marten's public API surface where possible for user familiarity diff --git a/src/Polecat.TestUtils/ConnectionSource.cs b/src/Polecat.TestUtils/ConnectionSource.cs index 504312d..a8ba431 100644 --- a/src/Polecat.TestUtils/ConnectionSource.cs +++ b/src/Polecat.TestUtils/ConnectionSource.cs @@ -1,7 +1,7 @@ -using Microsoft.Data.SqlClient; - -namespace Polecat.TestUtils; - +using Microsoft.Data.SqlClient; + +namespace Polecat.TestUtils; + /// /// Centralizes the SQL Server connection string for integration tests. /// Uses the POLECAT_TESTING_DATABASE environment variable if set, @@ -13,6 +13,42 @@ public static class ConnectionSource Environment.GetEnvironmentVariable("POLECAT_TESTING_DATABASE") ?? "Server=localhost,11433;User Id=sa;Password=P@55w0rd;Timeout=5;MultipleActiveResultSets=True;Initial Catalog=master;Encrypt=False"; + /// + /// The catalog this test process is pointed at — "master" by default, or whatever + /// POLECAT_TESTING_DATABASE names. + /// + public static string DatabaseName { get; } = + new SqlConnectionStringBuilder(ConnectionString).InitialCatalog; + + /// + /// A connection string for a different database on the same server. + /// + /// + /// Use this rather than ConnectionString.Replace("Initial Catalog=master", ...). + /// String replacement only works while the catalog happens to be "master": point + /// POLECAT_TESTING_DATABASE at anything else and the replace silently matches nothing, + /// so the "other" database quietly resolves to the current one and the test asserts + /// against itself. + /// + public static string ConnectionStringFor(string databaseName) => + new SqlConnectionStringBuilder(ConnectionString) { InitialCatalog = databaseName }.ConnectionString; + + /// Connection string for the server's own master catalog, for DDL. + public static string MasterConnectionString { get; } = ConnectionStringFor("master"); + + /// + /// A database name unique to this test process, so several processes can run the same + /// test class at once without fighting over one database. + /// + /// + /// Databases created by a test are siblings of the process's own database, not children + /// of it — so pointing each process at its own catalog does NOT isolate them. Anything a + /// test creates on the server has to carry the process's scope in its name, which is what + /// this does: Scoped("tenant_a") gives "master_tenant_a" locally and + /// "polecat_w3_tenant_a" under a parallel runner. + /// + public static string Scoped(string name) => $"{DatabaseName}_{name}"; + private static bool? _supportsNativeJson; /// diff --git a/src/Polecat.Tests/MultiTenancy/dynamic_tenant_source_tests.cs b/src/Polecat.Tests/MultiTenancy/dynamic_tenant_source_tests.cs index 0b53879..947769d 100644 --- a/src/Polecat.Tests/MultiTenancy/dynamic_tenant_source_tests.cs +++ b/src/Polecat.Tests/MultiTenancy/dynamic_tenant_source_tests.cs @@ -17,15 +17,14 @@ public class dynamic_tenant_source_tests : IAsyncLifetime { private const string TenantA = "tenant_a"; private const string TenantB = "tenant_b"; - private const string ControlDb = "polecat_dts_control"; - private const string DbA = "polecat_dts_tenant_a"; - private const string DbB = "polecat_dts_tenant_b"; + private static readonly string ControlDb = ConnectionSource.Scoped("dts_control"); + private static readonly string DbA = ConnectionSource.Scoped("dts_tenant_a"); + private static readonly string DbB = ConnectionSource.Scoped("dts_tenant_b"); - private static readonly string MasterConnectionString = - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", "Database=master"); + private static readonly string MasterConnectionString = ConnectionSource.MasterConnectionString; private static string Db(string name) => - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", $"Database={name}"); + ConnectionSource.ConnectionStringFor(name); public async ValueTask InitializeAsync() { diff --git a/src/Polecat.Tests/MultiTenancy/master_table_tenancy_tests.cs b/src/Polecat.Tests/MultiTenancy/master_table_tenancy_tests.cs index 4553127..f4c1bd3 100644 --- a/src/Polecat.Tests/MultiTenancy/master_table_tenancy_tests.cs +++ b/src/Polecat.Tests/MultiTenancy/master_table_tenancy_tests.cs @@ -11,15 +11,14 @@ public class master_table_tenancy_tests : IAsyncLifetime { private const string TenantA = "tenant_a"; private const string TenantB = "tenant_b"; - private const string ControlDb = "polecat_mt_control"; - private const string DbA = "polecat_mt_tenant_a"; - private const string DbB = "polecat_mt_tenant_b"; + private static readonly string ControlDb = ConnectionSource.Scoped("mt_control"); + private static readonly string DbA = ConnectionSource.Scoped("mt_tenant_a"); + private static readonly string DbB = ConnectionSource.Scoped("mt_tenant_b"); - private static readonly string MasterConnectionString = - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", "Database=master"); + private static readonly string MasterConnectionString = ConnectionSource.MasterConnectionString; private static string Db(string name) => - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", $"Database={name}"); + ConnectionSource.ConnectionStringFor(name); public async ValueTask InitializeAsync() { diff --git a/src/Polecat.Tests/MultiTenancy/separate_database_tenancy_tests.cs b/src/Polecat.Tests/MultiTenancy/separate_database_tenancy_tests.cs index 7db44d7..58d92e3 100644 --- a/src/Polecat.Tests/MultiTenancy/separate_database_tenancy_tests.cs +++ b/src/Polecat.Tests/MultiTenancy/separate_database_tenancy_tests.cs @@ -12,14 +12,13 @@ public class separate_database_tenancy_tests : IAsyncLifetime { private const string TenantA = "tenant_a"; private const string TenantB = "tenant_b"; - private const string DbA = "polecat_tenant_a"; - private const string DbB = "polecat_tenant_b"; + private static readonly string DbA = ConnectionSource.Scoped("tenant_a"); + private static readonly string DbB = ConnectionSource.Scoped("tenant_b"); - private static readonly string MasterConnectionString = - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", "Database=master"); + private static readonly string MasterConnectionString = ConnectionSource.MasterConnectionString; private static string TenantConnectionString(string dbName) => - ConnectionSource.ConnectionString.Replace("Initial Catalog=master", $"Database={dbName}"); + ConnectionSource.ConnectionStringFor(dbName); public async ValueTask InitializeAsync() {