From 06b3ea638f6881b0ef840e5a7f3356506b407c44 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Wed, 29 Apr 2020 18:37:21 -0300
Subject: [PATCH] Bug 25303: (QA follow-up) Fix test construction

The test called ->delete on the resultset directly, which is not the
same when we already called ->next and the Koha::Hold->delete won't be
called as there's no ->next hold.

I took the oportunity to wrap the subtest in its own transaction and
build its own data to avoid interference between tests...

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 t/db_dependent/Koha/Biblios.t | 75 ++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 32 deletions(-)

diff --git a/t/db_dependent/Koha/Biblios.t b/t/db_dependent/Koha/Biblios.t
index a6af46d288..dd26cf72de 100644
--- a/t/db_dependent/Koha/Biblios.t
+++ b/t/db_dependent/Koha/Biblios.t
@@ -62,38 +62,6 @@ subtest 'store' => sub {
     );
 };
 
-subtest 'holds + current_holds' => sub {
-    plan tests => 5;
-    C4::Reserves::AddReserve(
-        {
-            branchcode     => $patron->branchcode,
-            borrowernumber => $patron->borrowernumber,
-            biblionumber   => $biblio->biblionumber,
-        }
-    );
-    my $holds = $biblio->holds;
-    is( ref($holds), 'Koha::Holds', '->holds should return a Koha::Holds object' );
-    is( $holds->count, 1, '->holds should only return 1 hold' );
-    is( $holds->next->borrowernumber, $patron->borrowernumber, '->holds should return the correct hold' );
-    $holds->delete;
-
-    # Add a hold in the future
-    C4::Reserves::AddReserve(
-        {
-            branchcode       => $patron->branchcode,
-            borrowernumber   => $patron->borrowernumber,
-            biblionumber     => $biblio->biblionumber,
-            reservation_date => dt_from_string->add( days => 2 ),
-        }
-    );
-    $holds = $biblio->holds;
-    is( $holds->count, 1, '->holds should return future holds' );
-    $holds = $biblio->current_holds;
-    is( $holds->count, 0, '->current_holds should not return future holds' );
-    $holds->delete;
-
-};
-
 subtest 'waiting_or_in_transit' => sub {
     plan tests => 4;
     my $biblio = $builder->build( { source => 'Biblio' } );
@@ -222,3 +190,46 @@ subtest 'custom_cover_image_url' => sub {
 };
 
 $schema->storage->txn_rollback;
+
+subtest 'holds + current_holds' => sub {
+    plan tests => 5;
+
+    $schema->storage->txn_begin;
+
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    my $biblio = $builder->build_sample_biblio();
+
+    C4::Reserves::AddReserve(
+        {
+            branchcode     => $patron->branchcode,
+            borrowernumber => $patron->borrowernumber,
+            biblionumber   => $biblio->biblionumber,
+        }
+    );
+
+    my $holds = $biblio->holds;
+    is( ref($holds), 'Koha::Holds', '->holds should return a Koha::Holds object' );
+    is( $holds->count, 1, '->holds should only return 1 hold' );
+    is( $holds->next->borrowernumber, $patron->borrowernumber, '->holds should return the correct hold' );
+
+    # Start fresh
+    $biblio->holds->delete;
+
+    # Add a hold in the future
+    C4::Reserves::AddReserve(
+        {
+            branchcode       => $patron->branchcode,
+            borrowernumber   => $patron->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            reservation_date => dt_from_string->add( days => 2 ),
+        }
+    );
+    $holds = $biblio->holds;
+    is( $holds->count, 1, '->holds should return future holds' );
+    $holds = $biblio->current_holds;
+    is( $holds->count, 0, '->current_holds should not return future holds' );
+
+    $schema->storage->txn_rollback;
+};
+
+
-- 
2.26.2