Reviewer of Bundle C (#23 M1) flagged two blockers in the AddAuditLogTableMigration integration tests: 1. Tests used 'if (!await EnsureMigrationApplied()) return;' which made the xunit runner report them as Passed when the dev MSSQL container was absent — a CI false-positive risk. xunit 2.9.x does NOT ship the v3 Assert.Skip/SkipUnless/SkipWhen API surface (verified empirically against xunit.assert 2.9.3 — only v3.x exposes those static methods), so the canonical xunit-v2 equivalent is the Xunit.SkippableFact package. Replaced [Fact] with [SkippableFact] and the early-return pattern with 'Skip.IfNot(_fixture.Available, _fixture.SkipReason)' as the first statement of each of the 8 audit-log test methods. The runner now reports them as Skipped (not Passed) when MSSQL is down. 2. MsSqlMigrationFixture relied on SqlClient's 30s default connect timeout, so a no-container fixture construction hung ~30s. Added 'Connect Timeout=3' to DefaultAdminConnectionString. Verified fail-fast under ~4s end-to-end with a bad host via env-var override. Additional fixture cleanups: - Migration is now applied once in the fixture constructor (was per-test via EnsureMigrationApplied for idempotency). Tests reach a fully- migrated database with no extra setup. Removed the now-unused EnsureMigrationApplied helper from the test class. - Constructor narrowed its catch to SqlException + InvalidOperationException for the OpenAsync step (the only legitimate connect-failure surfaces); everything else (CREATE DATABASE, MigrateAsync) is treated as a hard fixture failure and bubbles up. Added a best-effort TryDropOrphanDatabase() pre-throw cleanup so partial construction cannot leak guid-suffixed databases. - Stale doc comments referencing the (non-existent) xunit 2.9.x Skip shim removed; replaced with accurate notes about Xunit.SkippableFact. Verified: - dotnet build ScadaLink.slnx: clean (0 warnings, 0 errors). - dotnet test ScadaLink.ConfigurationDatabase.Tests with MSSQL up: Passed 150 / Skipped 0 / Failed 0. - Same suite with SCADALINK_MSSQL_TEST_CONN pointed at a closed port: the 8 AddAuditLogTableMigration tests report as Skipped (visible '[SKIP]' lines in runner output), total elapsed ~3s. Files touched: - Directory.Packages.props: added Xunit.SkippableFact 1.5.61. - tests/ScadaLink.ConfigurationDatabase.Tests/ScadaLink.ConfigurationDatabase.Tests.csproj: added the SkippableFact PackageReference. - tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/MsSqlMigrationFixture.cs: Connect Timeout=3, constructor refactor, doc-comment fixes. - tests/ScadaLink.ConfigurationDatabase.Tests/Migrations/AddAuditLogTableMigrationTests.cs: [SkippableFact] + Skip.IfNot pattern across all 8 tests. Untouched (per reviewer guidance): - Migration file (Bundle C main artifact unchanged). - Bundle B reconciliation (composite PK + UX_AuditLog_EventId). - SqlClient VersionOverride 6.1.1 in the test csproj. - infra/* (separate uncommitted local edits remain in working tree).
242 lines
11 KiB
C#
242 lines
11 KiB
C#
using Microsoft.Data.SqlClient;
|
|
using Xunit;
|
|
|
|
namespace ScadaLink.ConfigurationDatabase.Tests.Migrations;
|
|
|
|
/// <summary>
|
|
/// Bundle C (#23 M1) integration tests: applies the EF migrations to a
|
|
/// freshly-created MSSQL test database on the running infra/mssql container
|
|
/// and asserts that the AddAuditLogTable migration produced the expected
|
|
/// partition function, partition scheme, partition-aligned table, named
|
|
/// indexes, and DB roles.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// Tests use <see cref="SkippableFactAttribute"/> + <c>Skip.IfNot(...)</c> 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
|
|
/// <c>Assert.Skip</c>/<c>Assert.SkipUnless</c> — those land in xunit v3 — so
|
|
/// SkippableFact is the canonical equivalent for this project. The fixture
|
|
/// applies the migration once at construction time.
|
|
/// </remarks>
|
|
public class AddAuditLogTableMigrationTests : IClassFixture<MsSqlMigrationFixture>
|
|
{
|
|
private readonly MsSqlMigrationFixture _fixture;
|
|
|
|
public AddAuditLogTableMigrationTests(MsSqlMigrationFixture fixture)
|
|
{
|
|
_fixture = fixture;
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesAuditLogTable()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var exists = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM INFORMATION_SCHEMA.TABLES " +
|
|
"WHERE TABLE_NAME = 'AuditLog' AND TABLE_SCHEMA = 'dbo';");
|
|
|
|
Assert.Equal(1, exists);
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesPartitionFunction_pf_AuditLog_Month()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var functionExists = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.partition_functions WHERE name = 'pf_AuditLog_Month';");
|
|
Assert.Equal(1, functionExists);
|
|
|
|
// Specification (alog.md §4 / Bundle C plan): 24 monthly boundaries
|
|
// covering 2026-01-01 through 2027-12-01 UTC.
|
|
var boundaryCount = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.partition_range_values rv " +
|
|
"INNER JOIN sys.partition_functions pf ON rv.function_id = pf.function_id " +
|
|
"WHERE pf.name = 'pf_AuditLog_Month';");
|
|
Assert.True(boundaryCount >= 24,
|
|
$"Expected at least 24 monthly boundaries on pf_AuditLog_Month; got {boundaryCount}.");
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesPartitionScheme_ps_AuditLog_Month()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var schemeExists = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.partition_schemes WHERE name = 'ps_AuditLog_Month';");
|
|
Assert.Equal(1, schemeExists);
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_TableIsPartitionAligned()
|
|
{
|
|
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.
|
|
var schemeName = await ScalarAsync<string?>(
|
|
"SELECT ps.name FROM sys.indexes i " +
|
|
"INNER JOIN sys.objects o ON i.object_id = o.object_id " +
|
|
"INNER JOIN sys.partition_schemes ps ON i.data_space_id = ps.data_space_id " +
|
|
"WHERE o.name = 'AuditLog' AND i.index_id = 1;");
|
|
|
|
Assert.Equal("ps_AuditLog_Month", schemeName);
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesFiveNamedIndexes()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var expected = new[]
|
|
{
|
|
"IX_AuditLog_OccurredAtUtc",
|
|
"IX_AuditLog_Site_Occurred",
|
|
"IX_AuditLog_CorrelationId",
|
|
"IX_AuditLog_Channel_Status_Occurred",
|
|
"IX_AuditLog_Target_Occurred",
|
|
};
|
|
|
|
foreach (var indexName in expected)
|
|
{
|
|
var count = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.indexes i " +
|
|
"INNER JOIN sys.objects o ON i.object_id = o.object_id " +
|
|
$"WHERE o.name = 'AuditLog' AND i.name = '{indexName}';");
|
|
Assert.True(count == 1, $"Expected index '{indexName}' to exist on AuditLog; found {count}.");
|
|
}
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesAuditWriterRole_WithExpectedGrants()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var roleExists = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_principals " +
|
|
"WHERE name = 'scadalink_audit_writer' AND type = 'R';");
|
|
Assert.Equal(1, roleExists);
|
|
|
|
// GRANT INSERT + GRANT SELECT must be present (G state = grant).
|
|
var insertGranted = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.objects o ON p.major_id = o.object_id " +
|
|
"WHERE pr.name = 'scadalink_audit_writer' AND o.name = 'AuditLog' " +
|
|
" AND p.permission_name = 'INSERT' AND p.state IN ('G','W');");
|
|
Assert.Equal(1, insertGranted);
|
|
|
|
var selectGranted = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.objects o ON p.major_id = o.object_id " +
|
|
"WHERE pr.name = 'scadalink_audit_writer' AND o.name = 'AuditLog' " +
|
|
" AND p.permission_name = 'SELECT' AND p.state IN ('G','W');");
|
|
Assert.Equal(1, selectGranted);
|
|
|
|
// UPDATE / DELETE must NOT be granted — and DENY (state = 'D') is even
|
|
// stronger. Treat presence of GRANT (state 'G' or 'W') as the failure.
|
|
var updateGranted = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.objects o ON p.major_id = o.object_id " +
|
|
"WHERE pr.name = 'scadalink_audit_writer' AND o.name = 'AuditLog' " +
|
|
" AND p.permission_name = 'UPDATE' AND p.state IN ('G','W');");
|
|
Assert.Equal(0, updateGranted);
|
|
|
|
var deleteGranted = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.objects o ON p.major_id = o.object_id " +
|
|
"WHERE pr.name = 'scadalink_audit_writer' AND o.name = 'AuditLog' " +
|
|
" AND p.permission_name = 'DELETE' AND p.state IN ('G','W');");
|
|
Assert.Equal(0, deleteGranted);
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AppliesMigration_CreatesAuditPurgerRole_WithExpectedGrants()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
var roleExists = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_principals " +
|
|
"WHERE name = 'scadalink_audit_purger' AND type = 'R';");
|
|
Assert.Equal(1, roleExists);
|
|
|
|
// SELECT on AuditLog.
|
|
var selectGranted = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.objects o ON p.major_id = o.object_id " +
|
|
"WHERE pr.name = 'scadalink_audit_purger' AND o.name = 'AuditLog' " +
|
|
" AND p.permission_name = 'SELECT' AND p.state IN ('G','W');");
|
|
Assert.Equal(1, selectGranted);
|
|
|
|
// ALTER on SCHEMA::dbo (class 3 = SCHEMA).
|
|
var alterSchema = await ScalarAsync<int>(
|
|
"SELECT COUNT(*) FROM sys.database_permissions p " +
|
|
"INNER JOIN sys.database_principals pr ON p.grantee_principal_id = pr.principal_id " +
|
|
"INNER JOIN sys.schemas s ON p.major_id = s.schema_id " +
|
|
"WHERE pr.name = 'scadalink_audit_purger' AND s.name = 'dbo' " +
|
|
" AND p.class = 3 AND p.permission_name = 'ALTER' AND p.state IN ('G','W');");
|
|
Assert.Equal(1, alterSchema);
|
|
}
|
|
|
|
[SkippableFact]
|
|
public async Task AuditWriterRole_CannotUpdateAuditLog()
|
|
{
|
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
|
|
|
// Set up a dedicated user mapped to scadalink_audit_writer, then EXECUTE AS
|
|
// and attempt UPDATE — DENY UPDATE on the role must reject the statement.
|
|
// Use a guid-suffixed user name so reruns in the same fixture don't collide.
|
|
var testUser = $"audit_writer_smoke_{Guid.NewGuid():N}".Substring(0, 32);
|
|
|
|
await using (var setup = new SqlConnection(_fixture.ConnectionString))
|
|
{
|
|
await setup.OpenAsync();
|
|
await using var setupCmd = setup.CreateCommand();
|
|
setupCmd.CommandText =
|
|
$"CREATE USER [{testUser}] WITHOUT LOGIN; " +
|
|
$"ALTER ROLE scadalink_audit_writer ADD MEMBER [{testUser}];";
|
|
await setupCmd.ExecuteNonQueryAsync();
|
|
}
|
|
|
|
var ex = await Assert.ThrowsAsync<SqlException>(async () =>
|
|
{
|
|
await using var conn = new SqlConnection(_fixture.ConnectionString);
|
|
await conn.OpenAsync();
|
|
await using var cmd = conn.CreateCommand();
|
|
// WHERE 1=0 guarantees no rows are touched even if the permission check
|
|
// somehow passes — the test asserts the engine rejects the statement
|
|
// at permission-check time, not via a side effect on data.
|
|
cmd.CommandText =
|
|
$"EXECUTE AS USER = '{testUser}'; " +
|
|
$"UPDATE dbo.AuditLog SET Status = 'X' WHERE 1 = 0; " +
|
|
$"REVERT;";
|
|
await cmd.ExecuteNonQueryAsync();
|
|
});
|
|
|
|
// SQL Server permission-denied errors carry number 229 (e.g. "The UPDATE
|
|
// permission was denied"). Assert the message mentions permission rather
|
|
// than pinning to the exact code, in case the engine version drifts.
|
|
Assert.Contains("permission", ex.Message, StringComparison.OrdinalIgnoreCase);
|
|
}
|
|
|
|
// --- helpers ------------------------------------------------------------
|
|
|
|
private async Task<T> ScalarAsync<T>(string sql)
|
|
{
|
|
await using var conn = _fixture.OpenConnection();
|
|
await using var cmd = conn.CreateCommand();
|
|
cmd.CommandText = sql;
|
|
var result = await cmd.ExecuteScalarAsync();
|
|
if (result is null || result is DBNull)
|
|
{
|
|
return default!;
|
|
}
|
|
return (T)Convert.ChangeType(result, typeof(T) == typeof(string) ? typeof(string) : Nullable.GetUnderlyingType(typeof(T)) ?? typeof(T))!;
|
|
}
|
|
}
|