test(transactions): verify consistency histories - #10875
Merged
ReubenBond merged 2 commits intoAug 28, 2026
Merged
Conversation
11 tasks
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — CheckConsistency mutates and relies on the instance fields 'orderEdges' and 'marks' but never… |
What changed in this PR
This PR adds deterministic BVT coverage for the Transactions TestKit’s consistency verifier and refactors ConsistencyTestHarness.CheckConsistency into smaller invariant-focused helpers so both randomized history collection and deterministic verification exercise the same code paths.
Changes:
- Added a new deterministic BVT test suite covering valid histories and specific inconsistency cases (missing versions, multiple writers, invalid readers/writers, dependency cycles, in-doubt and timeout tolerance).
- Refactored
ConsistencyTestHarnessto factor history recording into helpers and decompose consistency checking into smaller methods.
| File | Description |
|---|---|
| test/Transactions/Orleans.Transactions.Tests/ConsistencyTestHarnessTests.cs | New deterministic BVT tests for serialized history validation and rejection rules. |
| src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs | Refactors recording/checking into helpers and initializes output to a no-op by default. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
force-pushed
the
rb-issue-10863-raise-state-coverage
branch
from
August 28, 2026 01:00
4ff750e to
7f5b262
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 2
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — RecordSucceeded mutates harness state (adds to succeeded and starts recording observations) before… |
Pre-existing issues (1)
| Severity | Finding |
|---|---|
|
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — CheckConsistency mutates and relies on the instance fields 'orderEdges' and 'marks' but never… View comment |
Contributor
Code coverage78.65% line coverage - 99,638 / 126,681 lines Coverage details
|
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — CheckConsistency now only uses the method parameter tolerateUnknownExceptions, while the… |
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
|
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — RecordSucceeded mutates harness state (adds to succeeded and starts recording observations) before… View resolved comment |
|
src/Orleans.Transactions.TestKit.Base/Consistency/ConsistencyTestHarness.cs — CheckConsistency mutates and relies on the instance fields 'orderEdges' and 'marks' but never… View resolved comment |
Comment on lines
183
to
+187
| public void CheckConsistency(bool tolerateGenericTimeouts = false, bool tolerateUnknownExceptions = false) | ||
| { | ||
| orderEdges.Clear(); | ||
| marks.Clear(); | ||
|
|
This was referenced Aug 29, 2026
This was referenced Sep 7, 2026
This was referenced Sep 14, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The transaction TestKit consistency verifier had no deterministic coverage despite carrying the highest measured state-testing risk:
CheckConsistencywas uncovered with cyclomatic complexity 54 and CRAP 2970. Its rejection rules were exercised primarily through randomized and stress-oriented tests.Solution
Add deterministic BVT tests for valid serialized histories, missing versions, multiple and invalid writers, invalid readers, dependency cycles, in-doubt outcomes, and timeout tolerance. Factor history recording and consistency checks into invariant-specific helpers so runtime collection and deterministic verification use the same paths.
Rationale
The focused suite verifies persisted version observations, ordering, serializability, abort state, corruption-shaped histories, and recoverable incomplete outcomes without cloud dependencies. The extraction lowers
CheckConsistencycomplexity from 54 to 8 while focused coverage reaches 93.3% line and 100% branch for that method.Progress toward #10863.
Microsoft Reviewers: Open in CodeFlow