Repository navigation
test(postgresql): add Orleans smoke test pinned to OrleansVersion - #5
Merged
Merged
Conversation
- Reference the Orleans server and ADO.NET clustering, persistence and reminders packages at $(OrleansVersion) - Run an Orleans silo against a freshly migrated database and check that it joins and leaves the cluster, writes, reads and clears grain state with DeleteStateOnClear, and registers and unregisters reminders - Mention the smoke test in the README Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewer's GuideAdds a real Orleans silo smoke-test suite that migrates a fresh PostgreSQL database, exercises clustering, grain persistence, and reminders through all three ADO.NET providers, and verifies the resulting database state; the Orleans packages used by the test are pinned to the schema’s OrleansVersion. Sequence diagram for the Orleans PostgreSQL smoke testssequenceDiagram
participant Test as OrleansSmokeTests
participant DB as PostgreSQL
participant Silo as OrleansSilo
participant Cluster as IManagementGrain
participant Grain as SmokeGrain
participant Reminder as ReminderService
Test->>DB: MigrateAsync()
Test->>Silo: StartAsync()
Silo->>DB: MembershipTable
Test->>Cluster: GetHosts()
Cluster-->>Test: Active silo
Test->>Grain: SetState()
Grain->>DB: GrainStorage
Test->>Grain: GetState()
Grain-->>Test: Persisted state
Test->>Reminder: RegisterOrUpdateReminder()
Reminder->>DB: RemindersTable
Test->>Reminder: GetReminder()
Test->>Reminder: UnregisterReminder()
Reminder->>DB: Delete reminder row
Test->>Silo: StopAsync()
Silo->>DB: Mark silo dead
Entity relationship diagram for Orleans PostgreSQL smoke-test dataerDiagram
MEMBERSHIP_TABLE {
string silo_address
string status
}
GRAIN_STATE {
string grain_id
string state
}
REMINDERS_TABLE {
string grain_id
string reminder_name
}
ORLEANS_SILO ||--o| MEMBERSHIP_TABLE : registers
SMOKE_GRAIN ||--o| GRAIN_STATE : persists
SMOKE_GRAIN ||--o| REMINDERS_TABLE : schedules
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="test/PostgreSQL.Tests/Grains/SmokeGrain.cs" line_range="38" />
<code_context>
+
+ /// <inheritdoc />
+ public Task RegisterReminderAsync(string name) =>
+ this.RegisterOrUpdateReminder(name, TimeSpan.FromHours(1), TimeSpan.FromHours(1));
+
+ /// <inheritdoc />
</code_context>
<issue_to_address>
**nitpick (bug_risk):** RegisterReminderAsync ignores its `name` argument and always registers the reminder named `smoke`, so callers passing any other name cannot register the reminder they requested.
**Triggers:** When the smoke grain is called with a reminder name other than `smoke`.
**Suggested fix:** Pass `name` to `RegisterOrUpdateReminder` instead of the hard-coded `"smoke"` literal.
</issue_to_address>
### Comment 2
<location path="test/PostgreSQL.Tests/OrleansSmokeTests.cs" line_range="107" />
<code_context>
+ options.ClusterId = "smoke";
+ options.ServiceId = "smoke";
+ })
+ .ConfigureEndpoints(IPAddress.Loopback, GetFreePort(), GetFreePort())
+ .UseAdoNetClustering(options =>
+ {
</code_context>
<issue_to_address>
**issue (bug_risk):** The two endpoint ports are obtained from listeners which are disposed before Orleans binds them, and the two calls can return the same port or a port claimed by another process; Orleans then fails to start with an endpoint bind error.
**Triggers:** When the two free-port calls collide or another process binds a returned port before `host.StartAsync`.
**Suggested fix:** Reserve both ports until the silo has bound them, or use Orleans' dynamic endpoint-port configuration instead of probing ports with disposable listeners.
</issue_to_address>
### Comment 3
<location path="test/PostgreSQL.Tests/OrleansSmokeTests.cs" line_range="59" />
<code_context>
+ var grain = host.Services.GetRequiredService<IGrainFactory>().GetGrain<ISmokeGrain>("storage");
+
+ await grain.SetValueAsync("hello");
+ await grain.DeactivateAsync();
+
+ Assert.Equal("hello", await grain.GetValueAsync());
</code_context>
<issue_to_address>
**issue (testing):** `DeactivateOnIdle()` only schedules deactivation after the current grain call; awaiting `DeactivateAsync` does not wait until the grain has actually been deactivated. The immediately following `GetValueAsync` can therefore read the in-memory state, so the test passes without proving that state is reloaded from PostgreSQL after deactivation.
**Triggers:** When Orleans has not completed the scheduled deactivation before the next grain call is dispatched.
**Suggested fix:** Wait for observable deactivation and reactivation, or use a test hook/management API that confirms the grain has been removed before issuing the read.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: test/PostgreSQL.Tests/OrleansSmokeTests.cs:107, test/PostgreSQL.Tests/OrleansSmokeTests.cs:59
Test Results20 tests 20 ✅ 10s ⏱️ Results for commit fffc213. ♻️ This comment has been updated with latest results. |
- Pick the silo and gateway ports while both listeners are open, so they are always distinct - Assert that grain state is read back by a new activation, proving it was loaded from PostgreSQL rather than from memory Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
Adds a smoke test that runs a real Orleans silo against a freshly migrated database, using the Orleans version the schema claims to match. This is the optional item left over from #2.
Orleans packages (
Microsoft.Orleans.Server,.Clustering.AdoNet,.Persistence.AdoNet,.Reminders.AdoNet) are pinned to$(OrleansVersion)inDirectory.Packages.props, so bumpingOrleansVersionalso bumps the Orleans the tests run against.Three tests, each starting a silo that uses all three ADO.NET providers:
SiloJoinsAndLeavesClusterIManagementGrainand the membership table) and marked dead after shutdown.GrainStorageWritesReadsAndClearsDeleteStateOnClear).RemindersRegisterAndUnregisterREADME mentions the smoke test and that its Orleans packages follow
OrleansVersion.Notes
20230510224234), the silo fails to start with "DeleteStateOnClear=true is not supported … check persistence scripts". So the smoke test catches real Orleans incompatibilities, not just schema shape.global::Orleans…, because inside theEscendit.Orleans…namespace a plainOrleansresolves to the package's own namespace.Test plan
dotnet restore --locked-modeanddotnet build -c Releasepass with no warningsdotnet test -c Releaselocally: 20 passedubuntu-latest🤖 Generated with Claude Code
Summary by Sourcery
Validate PostgreSQL Orleans schema compatibility by exercising a real, OrleansVersion-matched silo across all ADO.NET providers.
New Features:
Enhancements:
Build:
Documentation:
Tests: