Bug 10499

Summary: UT: VirtualShelves.t needs a database transaction
Product: Koha Reporter: kenza <kenza.zaki>
Component: Test SuiteAssignee: kenza <kenza.zaki>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: gmcharlt, katrin.fischer, 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:
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

    
Comment 1 kenza 2013-06-20 13:20:17 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2013-06-20 13:52:55 UTC
Could you have a look at bug 10386? It was already there..
Comment 3 kenza 2013-06-21 09:38:57 UTC
The patch 10386 doesn't add the rollback, so i think these two patches are complementary.
Comment 4 kenza 2013-06-25 07:47:24 UTC
I just try the patch and it applies.
Comment 5 Chris Cormack 2013-06-25 08:05:53 UTC Comment hidden (obsolete)
Comment 6 kenza 2013-07-01 08:51:02 UTC Comment hidden (obsolete)
Comment 7 kenza 2013-07-01 09:04:28 UTC
This patch needs signoff again.In fact, the patch 10386 is pushed to master so i rebase this one.
Comment 8 Kyle M Hall 2013-07-02 14:00:19 UTC Comment hidden (obsolete)
Comment 9 Katrin Fischer 2013-07-14 07:37:24 UTC
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.
Comment 10 Katrin Fischer 2013-07-14 07:39:45 UTC
And welcome Kenza - thx for your patches :)
Comment 11 Galen Charlton 2013-07-16 14:16:15 UTC
(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.
Comment 12 Galen Charlton 2013-07-16 14:19:48 UTC
Pushed to master.  Thanks, Kenza!
Comment 13 Marcel de Rooy 2013-07-17 13:57:09 UTC
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.
Comment 14 Tomás Cohen Arazi 2013-07-25 17:14:48 UTC
This patch has been pushed to 3.12.x, will be in 3.12.3.

Thanks Kenza!