Add test for verifying exception handling - #585
Conversation
For in between transactions.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
|
|
||
| # The worker is idle here, so the cancel lands between transactions with | ||
| # nothing queued -- the state the fix is about. | ||
| sleep(2); |
There was a problem hiding this comment.
I think a sleep call here is a source of instability. I'd recommend using sync_event instead.
| # worker's exception log slot, which is what the incoming transaction is | ||
| # compared against after the cancel. | ||
| psql_or_bail(1, "INSERT INTO $t VALUES (1, 'before cancel')"); | ||
| is(wait_for_row($t, 1), '1', "$mode: baseline transaction replicates"); |
There was a problem hiding this comment.
I'd say it is too complicated. Use existing machinery: sync_event and table selection instead.
|
|
||
| psql_or_bail(2, "ALTER SYSTEM SET spock.exception_behaviour = $mode"); | ||
| psql_or_bail(2, "SELECT pg_reload_conf()"); | ||
| sleep(2); |
There was a problem hiding this comment.
That's a little annoying and delays tests. Maybe we can introduce a routine to apply extra parameters at instance start? It might help other tests to be a little more stable (and faster).
| @@ -0,0 +1,165 @@ | |||
| use strict; | |||
There was a problem hiding this comment.
Check if 037 is free to use in both main and v5 branches
| } | ||
|
|
||
| destroy_cluster('Destroy replay mode leak cluster'); | ||
| done_testing(); |
There was a problem hiding this comment.
Basically, nice sketch. Just make it simpler and introduce general mechanics into SpockTest.pm if needed - especially if vanilla's one already has something similar.
2 participants
For in between transactions.
This needs PR #584 to be merged first before the test passes, but the test is also purposely not in the CI schedule.