Bug 15081 - TestBuilder should not handle the transaction
Summary: TestBuilder should not handle the transaction
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Galen Charlton
URL:
Keywords:
Depends on:
Blocks: 14878
  Show dependency treegraph
 
Reported: 2015-10-29 13:43 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2017-06-14 22:05 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 15081: Take transaction handling out from TestBuilder (1.95 KB, patch)
2015-11-01 19:50 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 15081: (followup) Make test files using TestBuilder handle their transactions (18.94 KB, patch)
2015-11-01 19:50 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 15081: Take transaction handling out from TestBuilder (2.13 KB, patch)
2015-11-02 07:11 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15081: (followup) Make test files using TestBuilder handle their transactions (19.26 KB, patch)
2015-11-02 07:19 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 15081: Take transaction handling out from TestBuilder (2.19 KB, patch)
2015-11-04 14:34 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 15081: (followup) Make test files using TestBuilder handle their transactions (19.32 KB, patch)
2015-11-04 14:35 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 15081 [QA Followup] - Fix issue with Barcodes_ValueBuilder.t (806 bytes, patch)
2015-11-04 14:35 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2015-10-29 13:43:35 UTC
Right now, TestBuilder has the side effect of introducing a race condition on the DBIC transaction. The consecquences are uncomfortable, because warnings are raised on all tests that use it.

A patch removing it should make sure the tests have their own rollback handling using the Koha::Databases-provided schema.
Comment 1 Tomás Cohen Arazi (tcohen) 2015-11-01 19:50:39 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi (tcohen) 2015-11-01 19:50:43 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2015-11-02 07:11:07 UTC Comment hidden (obsolete)
Comment 4 Marc Véron 2015-11-02 07:19:35 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2015-11-02 07:24:04 UTC
Note: With and without patch, t/db_dependent/Barcodes_ValueBuilder.t fails with:

DBD::mysql::db do failed: Cannot delete or update a parent row: a foreign key constraint fails (`koha_kohadev`.`issues`, CONSTRAINT `issues_ibfk_2` FOREIGN KEY (`itemnumber`) REFERENCES `items` (`itemnumber`) ON UPDATE CASCADE) [for Statement "DELETE FROM items"] at t/db_dependent/Barcodes_ValueBuilder.t line 35.

not ok 2 - incremental barcode
Comment 6 Kyle M Hall (khall) 2015-11-04 14:34:52 UTC
Created attachment 44387 [details] [review]
[PASSED QA] Bug 15081: Take transaction handling out from TestBuilder

This patch removes transaction handling code from TestBuilder.
It fixes the TestBuilder.t to handle the transaction on its own.

Verify that t/db_dependent/TestBuilder.t passes.

Followed test plan, TestBuilder.t passes
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall (khall) 2015-11-04 14:35:02 UTC
Created attachment 44388 [details] [review]
[PASSED QA] Bug 15081: (followup) Make test files using TestBuilder handle their transactions

This trivial patch introduces the code needed on the test files so they handle
the DB transaction instead of relying on the (removed) transaction started/rolled back
by TestBuilder.

Tested all of the files before and after applying the patch, resultes are the same.
(Pass exept of t/db_dependent/Barcodes_ValueBuilder.t, this has the same error).
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2015-11-04 14:35:04 UTC
Created attachment 44389 [details] [review]
Bug 15081 [QA Followup] - Fix issue with Barcodes_ValueBuilder.t
Comment 9 Tomás Cohen Arazi (tcohen) 2015-11-04 15:55:39 UTC
Patches pushed to master.

Thanks for the followup, Kyle!
Comment 10 Frédéric Demians 2015-11-27 07:17:23 UTC
Is this patch needed on 3.20.x branch for its test suite?
Comment 11 Tomás Cohen Arazi (tcohen) 2015-11-27 12:15:40 UTC
(In reply to Frédéric Demians from comment #10)
> Is this patch needed on 3.20.x branch for its test suite?

I don't think so. It is part of a bigger tests rewrite. If it applies, it shouldn't harm and could help make jenkins stable for 3.20. But maybe it is not worth the trouble.