Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 18996: [QA Follow-up] Remove global variables from Message.t

Browse source 
As per request of Colin in comment18, this patch makes the use of global
variables in Message.t no longer needed.
The three subtests are now completely independent and could well be moved
to separate test scripts.

Note: Strictly speaking, the use of global (package) variables could
potentially introduce new bugs (e.g. if the value is modified outside the
script). This seems not to be the case here, but we are safe now.

Test plan:
Run the test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Colin Campbell <colin.campbell@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Marcel de Rooy 2017-08-02 13:01:47 +02:00 committed by Jonathan Druart
parent 2490401be1
commit 37d07b28ce
1 changed files with 49 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

75
t/db_dependent/SIP/Message.t
View file

@ -35,6 +35,7 @@ use Koha::DateUtils;
use Koha::Items;
use Koha::Checkouts;
use Koha::Old::Checkouts;
use Koha::Patrons;
use C4::SIP::ILS;
use C4::SIP::ILS::Patron;
@ -44,29 +45,9 @@ use C4::SIP::Sip::MsgType;
use constant PATRON_PW => 'do_not_ever_use_this_one';
our $fixed_length = { #length of fixed fields including response code
( PATRON_STATUS_RESP ) => 37,
( PATRON_INFO_RESP ) => 61,
( CHECKIN_RESP ) => 24,
};
our $schema = Koha::Database->new->schema;
our $builder = t::lib::TestBuilder->new();
# COMMON: Some common stuff for all/most subtests
our ( $response, $findpatron, $branchcode );
$branchcode = $builder->build({ source => 'Branch' })->{branchcode};
# mock write_msg (imported from Sip.pm into Message.pm)
my $mockMsg = Test::MockModule->new( 'C4::SIP::Sip::MsgType' );
$mockMsg->mock( 'write_msg', sub { $response = $_[1]; } ); # save response
# mock ils object
our $mockILS = Test::MockObject->new;
$mockILS->mock( 'check_inst_id', sub {} );
$mockILS->mock( 'institution_id', sub { $branchcode; } );
$mockILS->mock( 'find_patron', sub { $findpatron; } );
# START testing
subtest 'Testing Patron Status Request V2' => sub {
my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin;
plan tests => 13;
$C4::SIP::Sip::protocol_version = 2;
@ -75,6 +56,7 @@ subtest 'Testing Patron Status Request V2' => sub {
};
subtest 'Testing Patron Info Request V2' => sub {
my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin;
plan tests => 18;
$C4::SIP::Sip::protocol_version = 2;
@ -83,6 +65,7 @@ subtest 'Testing Patron Info Request V2' => sub {
};
subtest 'Checkin V2' => sub {
my $schema = Koha::Database->new->schema;
$schema->storage->txn_begin;
plan tests => 21;
$C4::SIP::Sip::protocol_version = 2;
@ -95,6 +78,11 @@ subtest 'Checkin V2' => sub {
# END of main code
sub test_request_patron_status_v2 {
my $builder = t::lib::TestBuilder->new();
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my ( $response, $findpatron );
my $mocks = create_mocks( \$response, \$findpatron, \$branchcode );
my $patron1 = $builder->build({
source => 'Borrower',
value => {
@ -111,7 +99,7 @@ sub test_request_patron_status_v2 {
FID_PATRON_PWD. PATRON_PW. '|';
my $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 );
my $server = { ils => $mockILS };
my $server = { ils => $mocks->{ils} };
undef $response;
$msg->handle_patron_status( $server );
@ -150,7 +138,7 @@ sub test_request_patron_status_v2 {
# Finally, we send a wrong card number and check AE, BL
# This is done by removing the new patron first
$schema->resultset('Borrower')->search({ cardnumber => $card1 })->delete;
Koha::Patrons->search({ cardnumber => $card1 })->delete;
undef $findpatron;
$siprequest = PATRON_STATUS_REQ. 'engYYYYMMDDZZZZHHMMSS'.
FID_INST_ID. $branchcode. '|'.
@ -166,6 +154,11 @@ sub test_request_patron_status_v2 {
}
sub test_request_patron_info_v2 {
my $builder = t::lib::TestBuilder->new();
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my ( $response, $findpatron );
my $mocks = create_mocks( \$response, \$findpatron, \$branchcode );
my $patron2 = $builder->build({
source => 'Borrower',
value => {
@ -181,7 +174,7 @@ sub test_request_patron_info_v2 {
FID_PATRON_PWD. PATRON_PW. '|';
my $msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 );
my $server = { ils => $mockILS };
my $server = { ils => $mocks->{ils} };
undef $response;
$msg->handle_patron_info( $server );
isnt( $response, undef, 'At least we got a response.' );
@ -228,7 +221,7 @@ sub test_request_patron_info_v2 {
check_field( $respcode, $response, FID_VALID_PATRON_PWD, 'Y', 'code CQ should be Y if empty AD allowed' );
# Finally, we send a wrong card number
$schema->resultset('Borrower')->search({ cardnumber => $card })->delete;
Koha::Patrons->search({ cardnumber => $card })->delete;
undef $findpatron;
$msg = C4::SIP::Sip::MsgType->new( $siprequest, 0 );
undef $response;
@ -240,6 +233,11 @@ sub test_request_patron_info_v2 {
}
sub test_checkin_v2 {
my $builder = t::lib::TestBuilder->new();
my $branchcode = $builder->build({ source => 'Branch' })->{branchcode};
my ( $response, $findpatron );
my $mocks = create_mocks( \$response, \$findpatron, \$branchcode );
# create some data
my $patron1 = $builder->build({
source => 'Borrower',
@ -255,6 +253,7 @@ sub test_checkin_v2 {
value => { damaged => 0, withdrawn => 0, itemlost => 0, restricted => 0, homebranch => $branchcode, holdingbranch => $branchcode },
});
my $mockILS = $mocks->{ils};
my $server = { ils => $mockILS, account => {} };
$mockILS->mock( 'institution', sub { $branchcode; } );
$mockILS->mock( 'supports', sub { return; } );
@ -342,12 +341,28 @@ sub test_checkin_v2 {
# Helper routines
sub create_mocks {
my ( $response, $findpatron, $branchcode ) = @_; # referenced variables !
# mock write_msg (imported from Sip.pm into Message.pm)
my $mockMsg = Test::MockModule->new( 'C4::SIP::Sip::MsgType' );
$mockMsg->mock( 'write_msg', sub { $$response = $_[1]; } ); # save response
# mock ils object
my $mockILS = Test::MockObject->new;
$mockILS->mock( 'check_inst_id', sub {} );
$mockILS->mock( 'institution_id', sub { $$branchcode; } );
$mockILS->mock( 'find_patron', sub { $$findpatron; } );
return { ils => $mockILS, message => $mockMsg };
}
sub check_field {
my ( $code, $resp, $fld, $expr, $msg, $mode ) = @_;
# mode: contains || equals || regex (by default: equals)
# strip fixed part; prefix to simplify next regex
$resp = '|'. substr( $resp, $fixed_length->{$code} );
$resp = '|'. substr( $resp, fixed_length( $code ) );
my $fldval;
if( $resp =~ /\|$fld([^\|]*)\|/ ) {
$fldval = $1;
@ -372,3 +387,11 @@ sub siprequestdate {
my ( $dt ) = @_;
return $dt->ymd('').(' 'x4).$dt->hms('');
}
sub fixed_length { #length of fixed fields including response code
return {
( PATRON_STATUS_RESP ) => 37,
( PATRON_INFO_RESP ) => 61,
( CHECKIN_RESP ) => 24,
}->{$_[0]};
}