#33161: Do not ignore transaction durability errors within TestCase
------------------------------------+--------------------------------------
     Reporter:  Krzysztof Jagiełło  |                    Owner:  nobody
         Type:  New feature         |                   Status:  new
    Component:  Testing framework   |                  Version:  3.2
     Severity:  Normal              |               Resolution:
     Keywords:                      |             Triage Stage:  Unreviewed
    Has patch:  1                   |      Needs documentation:  0
  Needs tests:  0                   |  Patch needs improvement:  0
Easy pickings:  0                   |                    UI/UX:  0
------------------------------------+--------------------------------------
Description changed by Krzysztof Jagiełło:

Old 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

New 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: https://github.com/django/django/pull/14919

--

-- 
Ticket URL: <https://code.djangoproject.com/ticket/33161#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/067.03795c5e32a4bf9ec2f8ed1a8240e549%40djangoproject.com.

Reply via email to