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?

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)'

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'

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.

(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.

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.

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 on attachment 45291 [details] [review]
Bug 15243 - Place a hold on... Fix display issue and improve translatability

Sorry, mistaked with bug number...

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>

Pushed to master!

Thanks Marc!

Patch pushed to 3.22.x, will be in 3.22.1

This patch has been pushed to 3.20.x, will be in 3.20.7.