test(adonet): raise relational provider coverage above 90% - #10892
Conversation
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageDataSourceTests.cs — In CreateInstance_AcceptsNativeNpgsqlDataSourceWithoutOpeningConnection, the connection string… |
|
test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageDataSourceTests.cs — ReadAsync_ReportsRecordsAffected is aggregating observations from the SELECT rows of a… |
What changed in this PR
Improves Orleans ADO.NET shared relational-provider reliability by significantly expanding deterministic unit/integration coverage for query loading/execution, parameter metadata, lifecycle/disposal behavior, and Orleans v3-compatible hashing, while removing an unused/fragile multi-row insert helper.
Changes:
- Add extensive new unit tests for
RelationalStorageExtensions,DbExtensions,RelationalOrleansQueries, and Orleans 3-compatible hashing. - Add SQLite in-memory lifecycle tests to validate result-set ordering, records-affected reporting, cancellation, and resource disposal.
- Forward explicit
DbTypewhen adding parameters and removeExecuteMultipleInsertIntoAsyncfrom shared relational extensions.
| File | Description |
|---|---|
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageExtensionsTests.cs | New tests for scripted storage read/execute behaviors, parameter forwarding, multi-result ordering, stream behavior. |
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageDataSourceTests.cs | Expanded lifecycle/disposal/cancellation tests using SQLite DbDataSource and additional CreateInstance validation. |
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalOrleansQueriesUnitTests.cs | New broad coverage for stored-query loading and provider query parameterization/mutation behavior. |
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/Orleans3CompatibleHasherTests.cs | New vector tests for Jenkins hash and Orleans v3 compatibility picker behavior. |
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/Fakes/ScriptedRelationalStorage.cs | New fake IRelationalStorage implementation for deterministic unit tests. |
| test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/DbExtensionsTests.cs | New tests covering parameter creation, reflection mapping, typed getters, and error paths. |
| src/AdoNet/Shared/Storage/RelationalStorageExtensions.cs | Remove unused ExecuteMultipleInsertIntoAsync implementation. |
| src/AdoNet/Shared/Storage/RelationalOrleansQueries.cs | Make CreateInstance(IRelationalStorage) internal to enable direct testing. |
| src/AdoNet/Shared/Storage/DbExtensions.cs | Ensure AddParameter forwards explicit DbType into parameter creation. |
| src/AdoNet/Orleans.Clustering.AdoNet/Orleans.Clustering.AdoNet.csproj | Add InternalsVisibleTo for Orleans.AdoNet.Tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: None
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageDataSourceTests.cs — ReadAsync_ReportsRecordsAffected is aggregating observations from the SELECT rows of a… View resolved comment |
|
test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageDataSourceTests.cs — In CreateInstance_AcceptsNativeNpgsqlDataSourceWithoutOpeningConnection, the connection string… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
test/Extensions/Orleans.AdoNet.Tests/StorageTests/Relational/RelationalStorageExtensionsTests.cs:121
- This test pins a NullReferenceException when
selectoris null, but the coreRelationalStorage.ReadAsyncimplementation validatesselectorand throwsArgumentNullException(seesrc/AdoNet/Shared/Storage/RelationalStorage.cs:200-203). Locking inNullReferenceExceptionis brittle and hides a parameter-validation bug in the convenience overload.
Consider aligning the extension overload to throw ArgumentNullException for a null selector (and updating this test accordingly) so behavior is consistent across IRelationalStorage entry points.
public async Task ReadAsync_RejectsNullSelector()
{
const string Sql = "SELECT Value FROM Sample";
var storage = new ScriptedRelationalStorage().ExpectRead(
Sql,
CreateTable(("Value", typeof(int), 42)));
var exception = await Assert.ThrowsAsync<NullReferenceException>(
() => storage.ReadAsync(Sql, (Func<IDataRecord, int>)null!, parameterProvider: null));
var call = Assert.Single(storage.Calls);
Assert.Equal(Sql, call.Query);
Assert.Null(exception.InnerException);
storage.VerifyComplete();
Code coverage78.74% line coverage - 99,625 / 126,528 lines Coverage details
|


Closes #10860
Raises deterministic coverage for shared ADO.NET relational behavior and Orleans 3-compatible hashing using fake ADO.NET infrastructure plus in-memory SQLite lifecycle tests.
The new tests pin stored-query loading, membership/reminder/stream/directory mutations, parameter metadata, cancellation and failure propagation, resource disposal, result-set ordering, and legacy Jenkins hash vectors. Physical linked-source coverage reaches 98.90% line / 96.51% branch for retained relational code and 99.16% line / 95.24% branch for the compatibility hash scope.
Also forwards explicit
DbTypemetadata when creating parameters. RemovesExecuteMultipleInsertIntoAsyncafter repository/API/history review found no callers and identified incompatible SQL behavior across supported dialects, including Oracle's requiredFROM DUAL; no replacement dialect contract is introduced.Microsoft Reviewers: Open in CodeFlow