Bugzilla – Attachment 65964 Details for
Bug 18996
SIP sets ok flag to true for refused checkin for data corruption
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
QA Remove global variables from patch
0004-Bug-18996-QA-Follow-up-Remove-global-variables-from-.patch (text/plain), 7.48 KB, created by
Colin Campbell
on 2017-08-14 16:48:27 UTC
(
hide
)
Description:
QA Remove global variables from patch
Filename:
MIME Type:
Creator:
Colin Campbell
Created:
2017-08-14 16:48:27 UTC
Size:
7.48 KB
patch
obsolete
>From 05a82d280f17c03f031638d84408ef86afed76a9 Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Wed, 2 Aug 2017 13:01:47 +0200 >Subject: [PATCH 4/4] Bug 18996: [QA Follow-up] Remove global variables from > Message.t > >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> >--- > t/db_dependent/SIP/Message.t | 75 +++++++++++++++++++++++++++++--------------- > 1 file changed, 49 insertions(+), 26 deletions(-) > >diff --git a/t/db_dependent/SIP/Message.t b/t/db_dependent/SIP/Message.t >index 50b2ef2670..12019c29bf 100755 >--- a/t/db_dependent/SIP/Message.t >+++ b/t/db_dependent/SIP/Message.t >@@ -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]}; >+} >-- >2.13.5 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 18996
:
65303
|
65304
|
65330
|
65336
|
65393
|
65394
|
65418
|
65961
|
65962
|
65963
|
65964
|
66180
|
66181
|
66182
|
66183
|
66184
|
67055
|
67056
|
67057
|
67058
|
67059
|
67849