Summary: | Prevent data loss when running Circulation.t without prove | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Test Suite | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andrewfh, martin.renvoize, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.04,21.05.13
|
|
Attachments: |
Bug 30203: Circulation.t: prevent locking by setting envvar
Bug 30203: Circulation.t: prevent locking by setting envvar |
Description
Marcel de Rooy
2022-03-01 13:13:08 UTC
Hmm. I should have known. One of the last subtests calls SendCirculationAlert.. And that sub includes one of the worst Koha hacks: # LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables. # If the LOCK/UNLOCK statements are executed from tests, the current transaction will be committed. # To avoid that we need to guess if this code is execute from tests or not (yes it is a bit hacky) my $do_not_lock = ( exists $ENV{_} && $ENV{_} =~ m|prove| ) || $ENV{KOHA_TESTING}; I will send a patch instead of closing this report btw. Created attachment 131314 [details] [review] Bug 30203: Circulation.t: prevent locking by setting envvar Running this test without prove is disastrous. The LOCK in the called SendCirculationAlert will ruin your data. But no longer when you apply this patch. Test plan: Run prove Circulation.t Inspect your data (e.g. borrowers table). Run perl Circulation.t Inspect your data (e.g. borrowers table) again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> This is a trivial edit but really handy to keep you focus on development instead of reinstalling or restoring db. Very trivial: Self SO Created attachment 131315 [details] [review] Bug 30203: Circulation.t: prevent locking by setting envvar Running this test without prove is disastrous. The LOCK in the called SendCirculationAlert will ruin your data. But no longer when you apply this patch. Test plan: Run prove Circulation.t Inspect your data (e.g. borrowers table). Run perl Circulation.t Inspect your data (e.g. borrowers table) again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Fix is clear and test plan works as expected. PQA Pushed to master for 22.05, thanks to everybody involved 🦄 I see this is in 21.11.x for 21.11.4 Pushed to 21.05.x for 21.05.13 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |