Summary: | UT: VirtualShelves.t needs a database transaction | ||
---|---|---|---|
Product: | Koha | Reporter: | kenza <kenza.zaki> |
Component: | Test Suite | Assignee: | kenza <kenza.zaki> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | P5 - low | CC: | gmcharlt, kenza.zaki, kyle, m.de.rooy, tomascohen |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 10290 | ||
Bug Blocks: | |||
Attachments: |
Bug 10499: VirtualShelves.t - wrap tests in a database connection
Bug 10499: VirtualShelves.t - wrap tests in a database connection Bug 10499: VirtualShelves.t - wrap tests in a database connection Bug 10499: VirtualShelves.t - wrap tests in a database connection [PASSED QA] Bug 10499: VirtualShelves.t - wrap tests in a database connection |
Description
kenza
2013-06-20 13:05:22 UTC
Created attachment 19171 [details] [review] Bug 10499: VirtualShelves.t - wrap tests in a database connection Before this patch, the queries in VirtualShelves.t were committed in the database and have to be removed at the end. This patch wraps tests in a database connection. Test plan: prove t/db_dependent/VirtualShelves.t VirtualShelves.t .. ok All tests successful. Files=1, Tests=72, 1 wallclock secs ( 0.06 usr 0.00 sys + 0.72 cusr 0.06 csys = 0.84 CPU) Result: PASS Could you have a look at bug 10386? It was already there.. The patch 10386 doesn't add the rollback, so i think these two patches are complementary. I just try the patch and it applies. Created attachment 19221 [details] [review] Bug 10499: VirtualShelves.t - wrap tests in a database connection Before this patch, the queries in VirtualShelves.t were committed in the database and have to be removed at the end. This patch wraps tests in a database connection. Test plan: prove t/db_dependent/VirtualShelves.t VirtualShelves.t .. ok All tests successful. Files=1, Tests=72, 1 wallclock secs ( 0.06 usr 0.00 sys + 0.72 cusr 0.06 csys = 0.84 CPU) Result: PASS Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Created attachment 19308 [details] [review] Bug 10499: VirtualShelves.t - wrap tests in a database connection Before this patch, the queries in VirtualShelves.t were committed in the database and have to be removed at the end. This patch wraps tests in a database connection. Test plan: prove t/db_dependent/VirtualShelves.t VirtualShelves.t .. ok All tests successful. Files=1, Tests=72, 1 wallclock secs ( 0.06 usr 0.00 sys + 0.72 cusr 0.06 csys = 0.84 CPU) Result: PASS This patch needs signoff again.In fact, the patch 10386 is pushed to master so i rebase this one. Created attachment 19343 [details] [review] Bug 10499: VirtualShelves.t - wrap tests in a database connection Before this patch, the queries in VirtualShelves.t were committed in the database and have to be removed at the end. This patch wraps tests in a database connection. Test plan: prove t/db_dependent/VirtualShelves.t VirtualShelves.t .. ok All tests successful. Files=1, Tests=72, 1 wallclock secs ( 0.06 usr 0.00 sys + 0.72 cusr 0.06 csys = 0.84 CPU) Result: PASS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 19619 [details] [review] [PASSED QA] Bug 10499: VirtualShelves.t - wrap tests in a database connection Before this patch, the queries in VirtualShelves.t were committed in the database and have to be removed at the end. This patch wraps tests in a database connection. Test plan: prove t/db_dependent/VirtualShelves.t VirtualShelves.t .. ok All tests successful. Files=1, Tests=72, 1 wallclock secs ( 0.06 usr 0.00 sys + 0.72 cusr 0.06 csys = 0.84 CPU) Result: PASS Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests pass. And welcome Kenza - thx for your patches :) (In reply to kenza from comment #3) > The patch 10386 doesn't add the rollback, so i think these two patches are > complementary. As a general note, it is not necessary to explicitly call $dbh->rollback. When AutoCommit is off, DBD::mysql automatically issues a rollback when the script ends, and if the script gets killed, the transaction never gets committed either. That said, it doesn't hurt anything to have the explicit rollback. Pushed to master. Thanks, Kenza! Although this patch is already pushed, I want to add some remarks: The code deletes the following lines: -for(my $i=0; $i<10;$i++){ - my $shelfnumber = $shelves[$i]->{number}; - if($shelfnumber<0) { - ok(1, 'Skip DelShelf for shelf -1'); - next; - } - my $status = DelShelf($shelfnumber); - ok(1 == $status, "deleted shelf $shelfnumber and its contents"); -} I do not agree with deleting those lines. Testing the DelShelf function should be done! Scrutiny :) I do agree with deleting this line: -DelBiblio($_) for @biblionumbers; I do not suggest to revert the patch however or send a followup. Why not? I am rebasing my patch on report 10441 that needs 9032 which is in Signed Off state. In this patch I will restore the DelShelf code. This patch has been pushed to 3.12.x, will be in 3.12.3. Thanks Kenza! |