diff --git a/Directory.Packages.props b/Directory.Packages.props index 21464c0..d8f854e 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -64,6 +64,14 @@ + + diff --git a/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/AddAuditLogTableMigrationTests.cs b/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/AddAuditLogTableMigrationTests.cs index 73e2359..159690a 100644 --- a/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/AddAuditLogTableMigrationTests.cs +++ b/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/AddAuditLogTableMigrationTests.cs @@ -1,5 +1,5 @@ using Microsoft.Data.SqlClient; -using Xunit.Abstractions; +using Xunit; namespace ScadaLink.ConfigurationDatabase.Tests.Migrations; @@ -11,28 +11,26 @@ namespace ScadaLink.ConfigurationDatabase.Tests.Migrations; /// indexes, and DB roles. /// /// -/// Tests early-return with a clear test-output message when the -/// reports unavailable, so CI without -/// the dev MSSQL container still runs the suite green. +/// Tests use + Skip.IfNot(...) from +/// the Xunit.SkippableFact package so the runner reports them as Skipped (not +/// Passed) when MSSQL is unreachable. xunit 2.9.x does not ship a native +/// Assert.Skip/Assert.SkipUnless — those land in xunit v3 — so +/// SkippableFact is the canonical equivalent for this project. The fixture +/// applies the migration once at construction time. /// public class AddAuditLogTableMigrationTests : IClassFixture { private readonly MsSqlMigrationFixture _fixture; - private readonly ITestOutputHelper _output; - public AddAuditLogTableMigrationTests(MsSqlMigrationFixture fixture, ITestOutputHelper output) + public AddAuditLogTableMigrationTests(MsSqlMigrationFixture fixture) { _fixture = fixture; - _output = output; } - [Fact] + [SkippableFact] public async Task AppliesMigration_CreatesAuditLogTable() { - if (!await EnsureMigrationApplied()) - { - return; - } + Skip.IfNot(_fixture.Available, _fixture.SkipReason); var exists = await ScalarAsync( "SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES " + @@ -41,13 +39,10 @@ public class AddAuditLogTableMigrationTests : IClassFixture( "SELECT COUNT(*) FROM sys.partition_functions WHERE name = 'pf_AuditLog_Month';"); @@ -63,26 +58,20 @@ public class AddAuditLogTableMigrationTests : IClassFixture( "SELECT COUNT(*) FROM sys.partition_schemes WHERE name = 'ps_AuditLog_Month';"); Assert.Equal(1, schemeExists); } - [Fact] + [SkippableFact] public async Task AppliesMigration_TableIsPartitionAligned() { - if (!await EnsureMigrationApplied()) - { - return; - } + Skip.IfNot(_fixture.Available, _fixture.SkipReason); // The clustered (PK) index on AuditLog must live on the ps_AuditLog_Month // partition scheme; sys.indexes.data_space_id points at the scheme. @@ -95,13 +84,10 @@ public class AddAuditLogTableMigrationTests : IClassFixture( "SELECT COUNT(*) FROM sys.database_principals " + @@ -171,13 +154,10 @@ public class AddAuditLogTableMigrationTests : IClassFixture( "SELECT COUNT(*) FROM sys.database_principals " + @@ -203,13 +183,10 @@ public class AddAuditLogTableMigrationTests : IClassFixture - /// Applies the migration to the per-fixture test database. Returns false - /// when the fixture is unavailable (no MSSQL container) — callers should - /// log + early-return so the test is reported green on a CI box without - /// the dev container. - /// - private async Task EnsureMigrationApplied() - { - if (!_fixture.Available) - { - _output.WriteLine($"[SKIP] {_fixture.SkipReason}"); - return false; - } - - // ApplyAuditMigrationAsync is idempotent — repeat calls within a fixture - // are a no-op after the first migration. Cheaper than re-creating the - // database per test for M1. - await _fixture.ApplyAuditMigrationAsync(); - return true; - } - private async Task ScalarAsync(string sql) { await using var conn = _fixture.OpenConnection(); diff --git a/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/MsSqlMigrationFixture.cs b/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/MsSqlMigrationFixture.cs index cca6093..e5906a0 100644 --- a/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/MsSqlMigrationFixture.cs +++ b/tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/MsSqlMigrationFixture.cs @@ -9,20 +9,29 @@ namespace ScadaLink.ConfigurationDatabase.Tests.Migrations; /// Creates a fresh, uniquely-named test database on the running infra/mssql /// container, applies the EF migrations against it, and drops it on dispose. /// When MSSQL is not reachable (CI without the container), -/// is set to false so each test can early-return cleanly — keeping the test -/// suite green wherever it runs. +/// is set to false and describes why — tests pair +/// [SkippableFact] with Skip.IfNot(_fixture.Available, _fixture.SkipReason) +/// so the runner reports them as Skipped (not silently Passed). /// /// -/// xUnit 2.9.x has no dynamic Skip; the early-return-after-output pattern is -/// the project convention. Tests calling -/// receive a clear log line in the output explaining why they did not run. +/// xunit 2.9.x has no native Assert.Skip/Assert.SkipUnless (those +/// are v3); the project uses the Xunit.SkippableFact package as the canonical +/// equivalent. The fixture attempts connect + create-db + migrate once at +/// construct time. The Connect Timeout=3 in +/// makes the fixture fail fast in a no-container environment (under ~5s total) +/// instead of hanging 30s on SqlClient's default. Only connect-failure exceptions +/// (SqlException, plus the InvalidOperationException SqlClient raises from +/// OpenAsync) flip Available to false — every other exception bubbles up so a +/// real bug is not silently swallowed. /// public sealed class MsSqlMigrationFixture : IDisposable { // Same credentials infra/mssql/setup.sql + docker-compose use. Not a committed // production secret — this is a local dev container connection string. + // Connect Timeout=3 makes the fixture fail fast (~3s) in a no-container + // environment rather than hanging on SqlClient's default 30s connect timeout. private const string DefaultAdminConnectionString = - "Server=localhost,1433;User Id=sa;Password=ScadaLink_Dev1#;TrustServerCertificate=true;Encrypt=false"; + "Server=localhost,1433;User Id=sa;Password=ScadaLink_Dev1#;TrustServerCertificate=true;Encrypt=false;Connect Timeout=3"; private const string AdminEnvVar = "SCADALINK_MSSQL_TEST_CONN"; @@ -58,29 +67,82 @@ public sealed class MsSqlMigrationFixture : IDisposable ? DefaultAdminConnectionString : fromEnv; + // Step 1: open the admin connection. This is the only step that may + // legitimately fail when MSSQL is absent; SqlException + the rare + // InvalidOperationException from OpenAsync are the connect-failure + // surfaces we tolerate. Everything else (CREATE DATABASE, MigrateAsync) + // is treated as a hard fixture failure once we *have* a connection. try { using var connection = new SqlConnection(_adminConnectionString); - // Short timeout so the suite skips quickly in a no-container environment - // rather than hanging on SqlClient's default 30s connect timeout. - connection.Open(); + try + { + connection.Open(); + } + catch (SqlException ex) + { + ConnectionString = string.Empty; + Available = false; + SkipReason = $"MSSQL unavailable (connect failed: SqlException {ex.Number}: {ex.Message})"; + return; + } + catch (InvalidOperationException ex) + { + ConnectionString = string.Empty; + Available = false; + SkipReason = $"MSSQL unavailable (OpenAsync threw: {ex.Message})"; + return; + } - using var createCmd = connection.CreateCommand(); - createCmd.CommandText = $"CREATE DATABASE [{DatabaseName}];"; - createCmd.ExecuteNonQuery(); + using (var createCmd = connection.CreateCommand()) + { + createCmd.CommandText = $"CREATE DATABASE [{DatabaseName}];"; + createCmd.ExecuteNonQuery(); + } ConnectionString = BuildPerDbConnectionString(_adminConnectionString, DatabaseName); + + // Apply the EF migrations once at fixture construction so each test + // can read from a fully-migrated database without per-test setup. + // Failures here are real bugs — let them bubble. + ApplyMigrationsCore(ConnectionString, CancellationToken.None).GetAwaiter().GetResult(); + Available = true; SkipReason = string.Empty; } - catch (Exception ex) + catch { - // Don't fail fixture construction — the surrounding test classes - // must remain runnable on a CI box without MSSQL. Each [Fact] gates - // on Available and skips with a clear reason via test output. - ConnectionString = string.Empty; - Available = false; - SkipReason = $"MSSQL not reachable at '{RedactPassword(_adminConnectionString)}': {ex.GetType().Name}: {ex.Message}"; + // Best-effort cleanup if we created the database but failed before + // setting Available — otherwise Dispose() would skip the drop. + TryDropOrphanDatabase(); + throw; + } + } + + private void TryDropOrphanDatabase() + { + if (string.IsNullOrEmpty(ConnectionString)) + { + return; + } + + try + { + SqlConnection.ClearAllPools(); + using var connection = new SqlConnection(_adminConnectionString); + connection.Open(); + using var cmd = connection.CreateCommand(); + cmd.CommandText = + $"IF DB_ID(N'{DatabaseName}') IS NOT NULL " + + $"BEGIN " + + $" ALTER DATABASE [{DatabaseName}] SET SINGLE_USER WITH ROLLBACK IMMEDIATE; " + + $" DROP DATABASE [{DatabaseName}]; " + + $"END"; + cmd.ExecuteNonQuery(); + } + catch + { + // Best-effort — orphan databases carry a random guid suffix. } } @@ -88,14 +150,13 @@ public sealed class MsSqlMigrationFixture : IDisposable /// Applies the EF migrations to the per-fixture test database via a freshly /// constructed pointed at it. Uses the /// schema-only single-argument constructor — the AuditLog migration does - /// not write secret-bearing columns at apply time. + /// not write secret-bearing columns at apply time. Called once from the + /// constructor; tests do not invoke this directly. /// - public async Task ApplyAuditMigrationAsync(CancellationToken cancellationToken = default) + private static async Task ApplyMigrationsCore(string connectionString, CancellationToken cancellationToken) { - ThrowIfUnavailable(); - var options = new DbContextOptionsBuilder() - .UseSqlServer(ConnectionString) + .UseSqlServer(connectionString) .Options; await using var context = new ScadaLinkDbContext(options); @@ -173,20 +234,4 @@ public sealed class MsSqlMigrationFixture : IDisposable }; return builder.ConnectionString; } - - private static string RedactPassword(string connectionString) - { - try - { - var builder = new SqlConnectionStringBuilder(connectionString) - { - Password = "***", - }; - return builder.ConnectionString; - } - catch - { - return ""; - } - } } diff --git a/tests/ScadaLink.ConfigurationDatabase.Tests/ScadaLink.ConfigurationDatabase.Tests.csproj b/tests/ScadaLink.ConfigurationDatabase.Tests/ScadaLink.ConfigurationDatabase.Tests.csproj index 15923fd..40997e3 100644 --- a/tests/ScadaLink.ConfigurationDatabase.Tests/ScadaLink.ConfigurationDatabase.Tests.csproj +++ b/tests/ScadaLink.ConfigurationDatabase.Tests/ScadaLink.ConfigurationDatabase.Tests.csproj @@ -26,6 +26,13 @@ + +