Opened 3 years ago
Last modified 3 years ago
#33161 closed New feature
Do not ignore transaction durability errors within TestCase — at Initial Version
Reported by: | Krzysztof Jagiełło | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | David Seddon, Adam Johnson, Ian Foote, Hannes Ljungberg | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently there is a discrepancy in how durable atomic blocks are handled in TransactionTestCase
vs TestCase
. Using the former, nested durable atomic blocks will, as expected, result in a RuntimeError
. Using the latter however, the error will go unnoticed as the durability check is turned off.
I have faced some issues with this behaviour in a codebase where we heavily utilize TestCase
and where we recently started to introduce durable atomic blocks – the durability errors do not surface until the code hits staging/production. The solution could be to switch over to TransactionTestCase
for the test classes that hit code paths with durable atomic blocks, but having to identify which tests could be affected by this issue is a bit inconvenient. And then there is the performance penalty of using TransactionTestCase
.
So, to the issue at hand. The durability check is disabled for TestCase
because otherwise durable atomic blocks would fail immediately as TestCase
wraps its tests in transactions. We could however add a marker to the transactions created by TestCase
, keep a stack of active transactions and make the durability check take the stack of transactions with their respective markers into account. This way we could easily detect when a durable atomic block is directly within a transaction created by TestCase
and skip the durability check only for this specific scenario.
To better illustrate what I am proposing here, I have prepared a PoC patch. Let me know what you think!
Patch: Coming soon