Enable Npgsql.Specification.Tests in the build pipeline#6504
Enable Npgsql.Specification.Tests in the build pipeline#6504roji merged 4 commits intonpgsql:mainfrom
Conversation
Skip specification tests where Npgsql's behavior is different than what the ADO.NET Specification Tests expect.
|
This is just an idea I had. Interested in your thoughts on this... |
There was a problem hiding this comment.
Pull request overview
Enables the Npgsql.Specification.Tests project to run in GitHub Actions CI, making ADO.NET specification alignment (and current divergences) part of the normal test signal without changing runtime behavior.
Changes:
- Run
test/Npgsql.Specification.Testsas part of the CI test step. - Override known failing spec tests and mark them as skipped with per-test reasons.
- Add xUnit analyzer suppression to allow intentional skips without failing the build.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
test/Npgsql.Specification.Tests/NpgsqlDataReaderTests.cs |
Adds skipped overrides for known DbDataReader spec divergences (exceptions/cancellation behavior). |
test/Npgsql.Specification.Tests/NpgsqlConnectionTests.cs |
Adds skipped overrides for connection disposal/event/cancellation divergences. |
test/Npgsql.Specification.Tests/NpgsqlCommandTests.cs |
Adds skipped overrides for command/transaction/parameter behavior divergences. |
.github/workflows/build.yml |
Runs dotnet test for the specification test project in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
roji
left a comment
There was a problem hiding this comment.
Thanks for the PR, please take a look at the comments.
… class * Assert expected base test failure when Npgsql behavior intentionally differs from the behavior tested by AdoNet.Specification.Tests * Combine all tests into a single dotnet test execuction in build.yml
There was a problem hiding this comment.
Pull request overview
This PR wires Npgsql.Specification.Tests into the CI signal by switching the GitHub Actions test step to run the full solution test set, and by explicitly documenting current ADO.NET specification divergences via skipped/overridden tests.
Changes:
- Run
dotnet testat the solution level in GitHub Actions (includingNpgsql.Specification.Tests), with a prerelease path filtering out plugin tests. - Add explicit per-test skips in
Npgsql.Specification.Teststo capture known divergences fromAdoNet.Specification.Tests. - Suppress xUnit analyzer warning
xUnit1004for intentionally skipped spec tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
test/Npgsql.Specification.Tests/NpgsqlDataReaderTests.cs |
Overrides and skips known-divergent DataReader spec tests with reasons. |
test/Npgsql.Specification.Tests/NpgsqlConnectionTests.cs |
Overrides and skips known-divergent Connection spec tests with reasons. |
test/Npgsql.Specification.Tests/NpgsqlCommandTests.cs |
Makes transaction-related divergences explicit and skips a couple additional divergent command tests. |
test/Npgsql.Specification.Tests/Npgsql.Specification.Tests.csproj |
Suppresses xUnit1004 to keep CI green with intentional skips. |
.github/workflows/build.yml |
Runs full-solution tests in CI and ensures the npgsql_tests database exists across OSes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
NinoFloris
left a comment
There was a problem hiding this comment.
LGTM, waiting for @roji to re-review. Thanks @brianpursley.
This enables
Npgsql.Specification.Testsin the GitHub Actions build.Today the specification test project exists in the repository but is not exercised by CI, which means provider/spec alignment is not visible in the normal test signal. This change brings that coverage into CI and makes the current state explicit.
Several tests are currently overridden and skipped with per-test reasons because Npgsql behavior differs from what
AdoNet.Specification.Testsexpects. These skips are intended to document the current state, not to declare that the current behavior is necessarily correct. Some of these differences may be by design, while others may represent compatibility gaps worth investigating separately.What this PR does:
test/Npgsql.Specification.Teststo the GitHub Actions test stepCurrent local result after this change:
217total197passing20skippedI think this provides a better baseline: