Repository navigation
Conversation
PR Summary by QodoPoll for PostgreSQL bookings before asserting projections
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1.
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
36dea06 to
ad8407a
Compare
|
Code review by qodo was updated up to the latest commit ad8407a |
Test Results 48 files ±0 48 suites ±0 15m 7s ⏱️ -8s Results for commit 5a7399b. ± Comparison against base commit ed6cfd1. This pull request removes 8 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
nmummau
changed the title
9c1e706 to
a301fd8
Compare
|
Code review by qodo was updated up to the latest commit a301fd8 |
Address Qodo's review finding that checkpoint timeouts could leave per-test hosts and subscriptions running. Wrap test operations in try/finally after successful host initialization so timeouts, cancellation, and assertion failures still call DisposeAsync(). Preserve assertions that must run after host shutdown. Verified all 9 MongoDB projection tests pass on .NET 10. Review: Eventuous#603 (comment)
|
Code review by qodo was updated up to the latest commit 87568cb |
Poll for projected bookings with a 30-second timeout and respect cancellation. Assert rows exist before checking their values. Fixes Eventuous#599
Address Qodo's review finding that checkpoint timeouts could leave per-test hosts and subscriptions running. Wrap test operations in try/finally after successful host initialization so timeouts, cancellation, and assertion failures still call DisposeAsync(). Preserve assertions that must run after host shutdown. Verified all 9 MongoDB projection tests pass on .NET 10. Review: Eventuous#603 (comment)
b88d5db to
7e86a3a
Compare
Address Qodo's review finding that checkpoint timeouts could leave per-test hosts and subscriptions running. Wrap test operations in try/finally after successful host initialization so timeouts, cancellation, and assertion failures still call DisposeAsync(). Preserve assertions that must run after host shutdown. Verified all 9 MongoDB projection tests pass on .NET 10. Review: Eventuous#603 (comment)
7e86a3a to
ba04816
Compare
|
Code review by qodo was updated up to the latest commit 969dda5 |
Poll for projected bookings with a 30-second timeout and respect cancellation. Assert rows exist before checking their values.
Add a 30-second timeout and propagate test cancellation. Report expected and observed event counts when polling times out.
WaitForPosition previously queried the checkpoint for ProjectWithBuilder regardless of the fixture's configured subscription ID. Bulk-builder and typed-handler tests therefore waited on another subscription, potentially returning too early or exhausting the polling loop unnecessarily. The helper also silently returned after 100 attempts, allowing document assertions to run without confirming that the projection had caught up. Use the fixture's configured subscription ID and replace the attempt limit with a 30-second timeout. Include the subscription ID, expected position, and last observed position in the timeout error. Propagate test cancellation through checkpoint reads, polling delays, and subsequent document reads. Preserve cancellation exceptions when the test itself is cancelled. Validation: all 9 MongoDB tests passed using MongoDB and KurrentDB Testcontainers.
CI failed Resubscribes_and_redelivers_the_event_the_handler_failed_on because the subscribed callback count was still 1 when the test expected at least 2. The logs showed the supervisor waiting for its one-second retry delay when test teardown stopped the subscription. The recovery wait checked only whether all events had been handled. RabbitMQ requeues the failed delivery before notifying the supervisor, so message recovery can complete before resubscription. Wait for all expected events, a drop callback, and a second subscribed callback within the existing 30-second timeout. Preserve external test cancellation and correct the comment claiming redelivery requires resubscription. Validation: all 5 RabbitMQ tests passed locally in Debug CI on net10.0.
CI repeatedly timed out waiting for redelivery after the subscription reported that it had resubscribed. Replace polling of handler completion signals with cancellation-aware task waits bounded to 30 seconds. Add a separate dispatch signal and phase-specific timeout messages to help locate recovery stalls. Always dispose subscriptions after assertions fail, and make the redelivery handler's success gate cancellable during cleanup. All 143 subscription tests pass on .NET 8 locally. The CI failure has not been reproduced locally, so its underlying cause remains unconfirmed.
Address Qodo's review finding that checkpoint timeouts could leave per-test hosts and subscriptions running. Wrap test operations in try/finally after successful host initialization so timeouts, cancellation, and assertion failures still call DisposeAsync(). Preserve assertions that must run after host shutdown. Verified all 9 MongoDB projection tests pass on .NET 10. Review: Eventuous#603 (comment)
Replace generated GUIDs and current timestamps in Service Bus serialization and sequence test data with fixed values. These arguments appear in reported test names, causing CI to count unchanged cases as removed and added on every run. Use UTC timestamps to keep the values consistent across time zones. Verified 26 serialization tests and 5 sequence tests pass. Fixes Eventuous#604
969dda5 to
03d926d
Compare
|
Code review by qodo was updated up to the latest commit 03d926d |
Import Eventuous.Tools and replace ConfigureAwait(false) with NoContext() in all three projection cleanup blocks to follow repository conventions. Verified the MongoDB test project builds on .NET 10.
|
Code review by qodo was updated up to the latest commit 7c427ff |
7c427ff to
5a7399b
Compare
|
@alexeyzimarev This is ready for a review and merge when you are ready. |
1 participant
Replace timing assumptions in asynchronous tests with bounded waits for observable progress, coordinate recovery scenarios explicitly, and keep parameterized test names stable between runs.
GetLastCheckpointcalls. Report the expected and last observed positions on timeout, and always stop initialized hosts infinallyblocks.Projection and metrics polling uses a 30-second bound and preserves external cancellation. PostgreSQL explicitly resolves the shared extension-method ambiguity so the new awaits can use
NoContext().The cancelled-message recovery failure was not reproduced locally; its changes improve synchronization, cleanup, and diagnostics without claiming a confirmed root-cause fix.
Local validation passed:
Fixes #599
Fixes #604