Speed up integration tests and fix a couple flaky ones#53
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the integration test infrastructure to reduce runtime by reusing a per-test-class database schema (migrated once) and resetting state between tests via table truncation, and it adjusts a couple incident-timeline integration tests to be less time-boundary flaky.
Changes:
- Rework
IntegrationTestBaseto cache a per-test-classNpgsqlDataSource+ schema, run migrations once, and truncate tables between tests. - Add an assembly-level cleanup hook to drop all test schemas and dispose their data sources after the test run.
- Stabilize incident timeline assertions by selecting the increment containing the result timestamp rather than assuming it’s the last increment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| SAMA.Tests.Integration/Web/Services/Queries/CheckQueryServiceTests.cs | Makes incident timeline tests less flaky by targeting the correct time increment. |
| SAMA.Tests.Integration/IntegrationTestBase.cs | Introduces per-class schema/data source reuse and per-test truncation to speed up integration tests. |
| SAMA.Tests.Integration/AssemblyHooks.cs | Adds assembly cleanup to drop created schemas and dispose shared data sources. |
| var guidHex = schemaName[(lastUnderscore + 1)..]; | ||
| if (guidHex.Length != 32 || !Guid.TryParse(guidHex, out _)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| // First 12 hex chars = 48 bits = unix timestamp in milliseconds | ||
| if (!long.TryParse(guidHex[..12], System.Globalization.NumberStyles.HexNumber, null, out var unixMs)) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| timestamp = DateTimeOffset.FromUnixTimeMilliseconds(unixMs); | ||
| return timestamp.Year is >= 2024 and <= 2100; |
There was a problem hiding this comment.
TryGetSchemaTimestamp only validates that the suffix is parseable as a GUID, but it doesn’t verify it’s actually UUIDv7 before interpreting the first 12 hex chars as a unix-ms timestamp. This can incorrectly classify non-v7 GUID suffixes as “stale” (by chance) and drop schemas that weren’t created by this test harness. Consider verifying the UUID version nibble is '7' (or equivalent check) before parsing/extracting the timestamp, and only then allow deletion.
| { | ||
| await using var dropCmd = conn.CreateCommand(); | ||
| dropCmd.CommandText = $"DROP SCHEMA IF EXISTS {schema} CASCADE"; | ||
| await dropCmd.ExecuteNonQueryAsync(); | ||
| } |
There was a problem hiding this comment.
DROP SCHEMA is built via string interpolation with an identifier coming from the database (pg_namespace). If a schema name ever contains characters that require quoting, this will fail; and interpolating identifiers directly is also avoidable risk. Consider quoting the identifier (e.g., via NpgsqlCommandBuilder.QuoteIdentifier) when constructing the DROP SCHEMA statement.
| await using var conn = await state.DataSource.OpenConnectionAsync(); | ||
| await using var cmd = conn.CreateCommand(); | ||
| cmd.CommandText = $"DROP SCHEMA IF EXISTS {state.SchemaName} CASCADE"; | ||
| await cmd.ExecuteNonQueryAsync(); | ||
| } |
There was a problem hiding this comment.
DROP SCHEMA is constructed with an unquoted schema identifier. Even though SchemaName is generated by the tests, quoting it makes the drop robust (and avoids edge-case failures if naming ever changes). Consider quoting the identifier when building the command text.
No description provided.