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.
Created attachment 44281 [details] [review] 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.
Created attachment 44282 [details] [review] 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.
Created attachment 44284 [details] [review] 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>
Created attachment 44286 [details] [review] 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>
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
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>
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>
Created attachment 44389 [details] [review] Bug 15081 [QA Followup] - Fix issue with Barcodes_ValueBuilder.t
Patches pushed to master. Thanks for the followup, Kyle!
Is this patch needed on 3.20.x branch for its test suite?
(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.