GH-4035: select the storage reset by its own marker instead of the durable-inbox one - #4038
Merged
Merged
Conversation
…rable-inbox one ClearAllWolverineStorageAsync() picked the queues it resets with `.Where(x => x is IDatabaseBackedEndpoint)`, so an endpoint's *inbox* behaviour silently decided whether integration tests could reset it. GH-4028 correctly removed that marker from RedisStreamEndpoint -- and thereby dropped Redis streams out of the reset entirely, with all 38 checks green, because no Redis implementation of the compliance suite existed. IStorageBackedQueue now says what the reset actually means: a queue whose contents live in storage Wolverine provisions rather than in an external broker. It is carried by the five database queues and by RedisStreamEndpoint, and it is independent of IDatabaseBackedEndpoint -- an endpoint may be either, both, or neither. Also fixed, found by the new coverage: * RedisStreamEndpoint.PurgeAsync() deleted the stream key but not the scheduled sorted set, so a scheduled message survived a "reset" and fired into the next test. The sorted set is part of this queue's storage exactly as the scheduled-message table is on the database queues. The compliance suite gains two seams so a non-table transport can implement it honestly rather than by bending its own surface to fit: * queueCountsAsync() is virtual -- RedisStreamEndpoint.GetAttributesAsync() reports streamKey/messageCount/consumerGroup, not the database queues' Count/Scheduled. * TeardownMakesTheQueueUnwritable gates the missing-storage precondition in rebuilds_queue_tables_that_have_been_dropped. A Redis XADD silently recreates a deleted stream key, so only the empties-it half of that scenario is observable there; the rest of the test still runs. Verified the new suite actually catches the regression: with the selector reverted to IDatabaseBackedEndpoint, 3 of its 5 tests fail. wolverine.slnx -c Release -f net9.0 clean. Wolverine.Redis.Tests 152/152. CoreTests 2525 (0 failed, 2 skipped). Sqlite and Postgresql clear_all_wolverine_storage 5/5 each. Co-Authored-By: Claude Opus 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.
Closes #4035. Follow-up to #4028 / #4031, which is merged and correct — this fixes a second-order consequence of it that CI could not see.
What broke
ClearAllWolverineStorageAsync()picked the queues it resets with.Where(x => x is IDatabaseBackedEndpoint), so an endpoint's inbox behaviour silently decided whether integration tests could reset it. #4031 correctly removed that marker fromRedisStreamEndpoint— and thereby dropped Redis streams out of the reset entirely. All 38 checks stayed green, because there was no Redis implementation ofClearAllWolverineStorageComplianceand no Redis test called the helper.The comment above the selector still claimed coverage of "PostgreSQL, SQL Server, MySQL, Oracle, SQLite, and Redis streams", and
docs/guide/testing.md:835presents the helper as the integration-test reset — so a Redis user following the docs started carrying stream entries between runs with no signal.The fix
IStorageBackedQueuesays what the reset actually means: a queue whose contents live in storage Wolverine provisions rather than in an external broker. Carried by the five database queues and byRedisStreamEndpoint, and independent ofIDatabaseBackedEndpoint— an endpoint may be either, both, or neither. Both interfaces now carry<remarks>saying which job is which, since conflating them is what caused this.A second bug, found by the new coverage
RedisStreamEndpoint.PurgeAsync()deleted the stream key but not the scheduled sorted set. So even once Redis was back in the reset path, a scheduled message survived a "reset" and fired into the next test. That sorted set is part of this queue's storage exactly as the scheduled-message table is on the database queues.Compliance suite seams
Two, so a non-table transport can implement the suite honestly instead of reshaping its own surface to fit:
queueCountsAsync()is virtual —RedisStreamEndpoint.GetAttributesAsync()reportsstreamKey/messageCount/consumerGroup, not the database queues'Count/Scheduled. Overriding beats changing a diagnostic surface other things read.TeardownMakesTheQueueUnwritablegates the missing-storage precondition inrebuilds_queue_tables_that_have_been_dropped. A RedisXADDsilently recreates a deleted stream key, so there is no absent "table" to observe; the rest of that test still runs for Redis.Verification
The new suite actually catches the regression — with the selector reverted to
IDatabaseBackedEndpoint, 3 of its 5 tests fail. A regression test that passes either way would have been worse than none, so this was checked rather than assumed.dotnet build wolverine.slnx -c Release -f net9.0Wolverine.Redis.TestsCoreTestsSqliteTests.Transport.clear_all_wolverine_storagePostgresqlTests.Transport.clear_all_wolverine_storageThe two existing provider suites are there to show the marker change didn't disturb them. All figures are from the current base (
d9a68b9ae, including #4032).🤖 Generated with Claude Code