Bug 15244 - t/db_dependent/Reserves.t depends on external data/configuration
Summary: t/db_dependent/Reserves.t depends on external data/configuration
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 15243
  Show dependency treegraph
 
Reported: 2015-11-23 20:21 UTC by Marc Véron
Modified: 2017-06-14 22:10 UTC (History)
7 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:


Attachments
Bug 15244: Fix failing test 14 in t/db_dependent/Reserves.t (1.39 KB, patch)
2015-12-01 19:49 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15244: t/db_dependent/Reserves.t depends on external data/configuration (1.44 KB, patch)
2015-12-01 19:52 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15243 - Place a hold on... Fix display issue and improve translatability (13.21 KB, patch)
2015-12-01 21:10 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15244: t/db_dependent/Reserves.t depends on external data/configuration (1.49 KB, patch)
2015-12-05 16:11 UTC, Galen Charlton
Details | Diff | Splinter Review
[PASSED QA] Bug 15244: t/db_dependent/Reserves.t depends on external data/configuration (1.55 KB, patch)
2015-12-06 21:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2015-11-23 20:21:32 UTC
Running t/db_dependent/Reserves.t results in:

not ok 14 - for generous library, its items fill first hold request in line (bug 10272)

#   Failed test 'for generous library, its items fill first hold request in line (bug 10272)'
#   at t/db_dependent/Reserves.t line 284.
#          got: '276'
#     expected: '277'
Comment 1 Jesse Weaver 2015-12-01 16:21:12 UTC
Cannot duplicate on latest master:

[elf]:~/p/koha> prove t/db_dependent/Reserves.t -- [2015-12-01 09:12] -----------------------------------------------------------------------------------------------------------------------
t/db_dependent/Reserves.t .. 1/73 Subroutine C4::Context::userenv redefined at t/db_dependent/Reserves.t line 44.
t/db_dependent/Reserves.t .. 2/73 C4::Context->userenv not defined! at /home/feoh3/p/koha/C4/Items.pm line 2336.
t/db_dependent/Reserves.t .. ok     
All tests successful.
Files=1, Tests=73,  3 wallclock secs ( 0.03 usr  0.00 sys +  1.34 cusr  0.06 csys =  1.43 CPU)
Result: PASS

That's even more worrisome, though. Marc, what's confusing to me is that borrower 277 shouldn't exist, from my reading of the test. $requestors{RPL} is the last borrower to be added. If this is reproducible, could you add in a:

warn Dumper( GetMember( borrowernumber => $messages->{ResFound}->{borrowernumber} ) );

just before the failing test?
Comment 2 Marc Véron 2015-12-01 16:35:14 UTC
With additional line 280:

warn Dumper( GetMember( borrowernumber => $messages->{ResFound}->{borrowernumber} ) );

...I get...
ok 13 - restrictive library's items only fill requests by own patrons (bug 10272)
$VAR1 = {
          'category_type' => 'A',
          'categorycode' => 'PT',
          'B_address' => undef,
          'ethnicity' => undef,
          'contactnote' => undef,
          'email' => undef,
          'password' => '!',
          'B_country' => undef,
          'borrowernumber' => '267',
          'branchcode' => 'CPL',
          'lost' => undef,
          'description' => 'Erwachsener',
          'streettype' => undef,
          'altcontactaddress3' => undef,
          'contactfirstname' => undef,
          'title' => undef,
          'ethnotes' => undef,
          'relationship' => undef,
          'mobile' => undef,
          'fax' => undef,
          'primary_contact_method' => undef,
          'altcontactphone' => undef,
          'contactname' => undef,
          'country' => undef,
          'dateenrolled' => '2015-11-25',
          'altcontactstate' => undef,
          'guarantorid' => undef,
          'address2' => undef,
          'borrowernotes' => undef,
          'dateexpiry' => '2016-11-25',
          'sort2' => undef,
          'contacttitle' => undef,
          'phonepro' => undef,
          'smsalertnumber' => undef,
          'B_streetnumber' => undef,
          'emailpro' => undef,
          'altcontactcountry' => undef,
          'firstname' => undef,
          'gonenoaddress' => undef,
          'othernames' => undef,
          'dateofbirth' => undef,
          'state' => undef,
          'altcontactaddress2' => undef,
          'B_streettype' => undef,
          'debarred' => undef,
          'B_state' => undef,
          'B_address2' => undef,
          'address' => '',
          'privacy' => '1',
          'streetnumber' => undef,
          'surname' => 'borrower from CPL',
          'cardnumber' => undef,
          'altcontactsurname' => undef,
          'altcontactzipcode' => undef,
          'altcontactfirstname' => undef,
          'opacnote' => undef,
          'userid' => 'borrowerfromcpl',
          'B_zipcode' => undef,
          'B_email' => undef,
          'city' => '',
          'B_phone' => undef,
          'debarredcomment' => undef,
          'initials' => undef,
          'sort1' => undef,
          'flags' => undef,
          'zipcode' => undef,
          'phone' => undef,
          'sex' => undef,
          'altcontactaddress1' => undef,
          'B_city' => undef
        };
not ok 14 - for generous library, its items fill first hold request in line (bug 10272)

#   Failed test 'for generous library, its items fill first hold request in line (bug 10272)'
Comment 3 Marc Véron 2015-12-01 17:10:28 UTC
If I dump $messages (line 284)
(undef, $messages, undef, undef) = AddReturn('bug10272_FPL', 'FPL');
warn Dumper( $messages );


...I get...:
$VAR1 = {
          'NotIssued' => 'bug10272_FPL',
          'ResFound' => {
                          'itemnumber' => '295',
                          'priority' => 2,
                          'reservenotes' => '',
                          'ResFound' => 'Reserved',
                          'reservedate' => '2015-12-01',
                          'found' => undef,
                          'timestamp' => '2015-12-01 18:04:24',
                          'biblionumber' => '45049',
                          'waitingdate' => undef,
                          'cancellationdate' => undef,
                          'reserve_id' => '151',
                          'borrowernumber' => '276',
                          'branchcode' => 'FPL'
                        }
        };
That means that the item was not issued? - Test expects 'ResFound'
Comment 4 Marc Véron 2015-12-01 17:31:05 UTC
Ok previous comment was a mistake.

I added the following after AddReturn (line 285):
warn Dumper( $messages );
warn Dumper ($messages->{ResFound}->{borrowernumber});
warn Dumper ($requesters{'RPL'});


And I get:
$VAR1 = {
          'NotIssued' => 'bug10272_FPL',
          'ResFound' => {
                          'itemnumber' => '301',
                          'priority' => 2,
                          'reservenotes' => '',
                          'ResFound' => 'Reserved',
                          'reservedate' => '2015-12-01',
                          'found' => undef,
                          'timestamp' => '2015-12-01 18:25:57',
                          'biblionumber' => '45053',
                          'waitingdate' => undef,
                          'cancellationdate' => undef,
                          'reserve_id' => '203',
                          'borrowernumber' => '292',
                          'branchcode' => 'FPL'
                        }
        };
$VAR1 = '292';
$VAR1 = '293';


...and for the next run it is
$VAR1 = '300';
$VAR1 = '301';

Hmm, is the database not properly reset after the tests? The borrower numbers increase with every run.
Comment 5 Tomás Cohen Arazi 2015-12-01 18:15:57 UTC
(In reply to Marc Véron from comment #4)
> $VAR1 = '292';
> $VAR1 = '293';
> 
> 
> ...and for the next run it is
> $VAR1 = '300';
> $VAR1 = '301';
> 
> Hmm, is the database not properly reset after the tests? The borrower
> numbers increase with every run.

That's because the rollback is not resetting the AUTO_INCREMENT fields counter. MySQL behaviour.
Comment 6 Marc Véron 2015-12-01 19:31:10 UTC
Found after systematically switching sysprefs:

Syspref LocalHoldsPriority was set to "[Give] priority for filling holds to patrons whose...

If I set it to [Don't give] the test passes OK.
Comment 7 Tomás Cohen Arazi 2015-12-01 19:47:11 UTC
Maybe the bug could be named "t/db_dependent/Reserves.t depends on external data/configuration" as the problem here is that the test doesn't set its needed variables before running.
Comment 8 Marc Véron 2015-12-01 19:49:20 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2015-12-01 19:52:42 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2015-12-01 21:10:02 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2015-12-01 21:11:01 UTC
Comment on attachment 45291 [details] [review]
Bug 15243 - Place a hold on... Fix display issue and improve translatability

Sorry, mistaked with bug number...
Comment 12 Galen Charlton 2015-12-05 16:11:08 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2015-12-06 21:26:08 UTC
Created attachment 45437 [details] [review]
[PASSED QA] Bug 15244: t/db_dependent/Reserves.t depends on external data/configuration

Test 14 in t/db_dependent/Reserves.t fails if syspref LocalHoldsPriority is
set to "[Give] priority for filling holds to patrons whose...

To verify:
- Set syspref as above and run test. Test 14 fails.
- Set syspref to [Don't give] and run test agein, it passes OK.

To test:
- Apply patch
- Run test again with syspref set / not set
Expected result: In both cases test should pass

(AMended to change bug title)

Signed-off-by: Galen Charlton <gmcharlt@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Kyle M Hall 2015-12-11 16:10:27 UTC
Pushed to master!

Thanks Marc!
Comment 15 Julian Maurice 2015-12-14 07:16:56 UTC
Patch pushed to 3.22.x, will be in 3.22.1
Comment 16 Frédéric Demians 2015-12-14 17:57:49 UTC
This patch has been pushed to 3.20.x, will be in 3.20.7.