Bugzilla – Attachment 59337 Details for
Bug 17909
Add unit tests for authority merge
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 17909: Additional polishing
Bug-17909-Additional-polishing.patch (text/plain), 12.32 KB, created by
Julian Maurice
on 2017-01-20 11:25:05 UTC
(
hide
)
Description:
Bug 17909: Additional polishing
Filename:
MIME Type:
Creator:
Julian Maurice
Created:
2017-01-20 11:25:05 UTC
Size:
12.32 KB
patch
obsolete
>From 87a2714a3983273bafc834645995dd50c39a491b Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Mon, 16 Jan 2017 12:15:56 +0100 >Subject: [PATCH] Bug 17909: Additional polishing > >No spectacular things: > >[1] Move the framework modifications to a sub. Use same style to create auth types and linked fields. >[2] Change some new Object occurrences to Object->new. >[3] Add tests for field count and field order in the first two subsets. >[4] Few whitespace changes (sorry) and comment lines. > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> > >Signed-off-by: Josef Moravec <josef.moravec@gmail.com> >Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> >--- > t/db_dependent/Authorities/Merge.t | 185 ++++++++++++++++++++++--------------- > 1 file changed, 108 insertions(+), 77 deletions(-) > >diff --git a/t/db_dependent/Authorities/Merge.t b/t/db_dependent/Authorities/Merge.t >index 9b811151a4..123577e381 100755 >--- a/t/db_dependent/Authorities/Merge.t >+++ b/t/db_dependent/Authorities/Merge.t >@@ -35,85 +35,86 @@ my $zoom_obj = Test::MockObject->new; > my $zoom_record_obj = Test::MockObject->new; > set_mocks(); > >+# Framework operations >+my ( $authtype1, $authtype2 ) = modify_framework(); >+ > subtest 'Test merge A1 to A2 (withing same authtype)' => sub { > # Tests originate from bug 11700 >- plan tests => 5; >- >- # Create authority type TEST_PERSO >- $dbh->do("INSERT INTO auth_types(authtypecode, authtypetext, auth_tag_to_report, summary) VALUES('TEST_PERSO', 'Personal Name', '109', 'Personal Names');"); >- $dbh->do("INSERT INTO auth_tag_structure (authtypecode, tagfield, liblibrarian, libopac, repeatable, mandatory, authorised_value) VALUES('TEST_PERSO', '109', 'HEADING--PERSONAL NAME', 'HEADING--PERSONAL NAME', 0, 0, NULL)"); >- $dbh->do("INSERT INTO auth_subfield_structure (authtypecode, tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, tab, authorised_value, value_builder, seealso, isurl, hidden, linkid, kohafield, frameworkcode) VALUES ('TEST_PERSO', '109', 'a', 'Personal name', 'Personal name', 0, 0, 1, NULL, NULL, '', 0, 0, '', '', '')"); >- >- my $auth1 = new MARC::Record; >- $auth1->append_fields(new MARC::Field('109', '0', '0', 'a' => 'George Orwell')); >- my $authid1 = AddAuthority($auth1, undef, 'TEST_PERSO'); >- my $auth2 = new MARC::Record; >- $auth2->append_fields(new MARC::Field('109', '0', '0', 'a' => 'G. Orwell')); >- my $authid2 = AddAuthority($auth2, undef, 'TEST_PERSO'); >- >- $dbh->do("INSERT IGNORE INTO marc_subfield_structure(tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, kohafield, tab, authorised_value, authtypecode, value_builder, isurl, hidden, frameworkcode, seealso, link, defaultvalue) VALUES('609', 'a', 'Personal name', 'Personal name', 0, 0, '', 6, '', 'TEST_PERSO', '', NULL, 0, '', '', '', NULL)"); >- $dbh->do("UPDATE marc_subfield_structure SET authtypecode = 'TEST_PERSO' WHERE tagfield='609' AND tagsubfield='a' AND frameworkcode='';"); >- my $tagfields = $dbh->selectcol_arrayref("select distinct tagfield from marc_subfield_structure where authtypecode='TEST_PERSO'"); >- my $biblio1 = new MARC::Record; >- $biblio1->append_fields( >- new MARC::Field('609', '0', '0', '9' => $authid1, 'a' => 'George Orwell') >- ); >+ plan tests => 9; >+ >+ # Add two authority records >+ my $auth1 = MARC::Record->new; >+ $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell' )); >+ my $authid1 = AddAuthority( $auth1, undef, $authtype1 ); >+ my $auth2 = MARC::Record->new; >+ $auth2->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'G. Orwell' )); >+ my $authid2 = AddAuthority( $auth2, undef, $authtype1 ); >+ >+ # Add two biblio records >+ my $biblio1 = MARC::Record->new; >+ $biblio1->append_fields( MARC::Field->new( '609', '0', '0', '9' => $authid1, 'a' => 'George Orwell' )); > my ( $biblionumber1 ) = AddBiblio($biblio1, ''); >- my $biblio2 = new MARC::Record; >- $biblio2->append_fields( >- new MARC::Field('609', '0', '0', '9' => $authid2, 'a' => 'G. Orwell') >- ); >+ my $biblio2 = MARC::Record->new; >+ $biblio2->append_fields( MARC::Field->new( '609', '0', '0', '9' => $authid2, 'a' => 'G. Orwell' )); > my ( $biblionumber2 ) = AddBiblio($biblio2, ''); > >+ # Time to merge > @zebrarecords = ( $biblio1, $biblio2 ); > $index = 0; > my $rv = C4::AuthoritiesMarc::merge( $authid2, $auth2, $authid1, $auth1 ); > is( $rv, 1, 'We expect one biblio record (out of two) to be updated' ); > >- $biblio1 = GetMarcBiblio($biblionumber1); >- is($biblio1->subfield('609', '9'), $authid1, 'Check biblio1 609$9' ); >- is($biblio1->subfield('609', 'a'), 'George Orwell', >+ # Check the results >+ my $newbiblio1 = GetMarcBiblio($biblionumber1); >+ compare_field_count( $biblio1, $newbiblio1, 1 ); >+ compare_field_order( $biblio1, $newbiblio1, 1 ); >+ is( $newbiblio1->subfield('609', '9'), $authid1, 'Check biblio1 609$9' ); >+ is( $newbiblio1->subfield('609', 'a'), 'George Orwell', > 'Check biblio1 609$a' ); >- $biblio2 = GetMarcBiblio($biblionumber2); >- is($biblio2->subfield('609', '9'), $authid1, 'Check biblio2 609$9' ); >- is($biblio2->subfield('609', 'a'), 'George Orwell', >+ my $newbiblio2 = GetMarcBiblio($biblionumber2); >+ compare_field_count( $biblio2, $newbiblio2, 1 ); >+ compare_field_order( $biblio2, $newbiblio2, 1 ); >+ is( $newbiblio2->subfield('609', '9'), $authid1, 'Check biblio2 609$9' ); >+ is( $newbiblio2->subfield('609', 'a'), 'George Orwell', > 'Check biblio2 609$a' ); > }; > > subtest 'Test merge A1 to modified A1' => sub { > # Tests originate from bug 11700 >- plan tests => 4; >- >- $dbh->do("INSERT IGNORE INTO marc_subfield_structure(tagfield, tagsubfield, liblibrarian, libopac, repeatable, mandatory, kohafield, tab, authorised_value, authtypecode, value_builder, isurl, hidden, frameworkcode, seealso, link, defaultvalue) VALUES('109', 'a', 'Personal name', 'Personal name', 0, 0, '', 6, '', 'TEST_PERSO', '', NULL, 0, '', '', '', NULL)"); >- $dbh->do("UPDATE marc_subfield_structure SET authtypecode = 'TEST_PERSO' WHERE tagfield='109' AND tagsubfield='a' AND frameworkcode='';"); >+ plan tests => 8; > >+ # Simulate modifying an authority from auth1old to auth1new > my $auth1old = MARC::Record->new; > $auth1old->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'Bruce Wayne' )); > my $auth1new = $auth1old->clone; > $auth1new->field('109')->update( a => 'Batman' ); >- my $authid1 = AddAuthority( $auth1new, undef, 'TEST_PERSO' ); >+ my $authid1 = AddAuthority( $auth1new, undef, $authtype1 ); > >- my $MARC1 = MARC::Record->new(); >- $MARC1->append_fields( MARC::Field->new( '245', '', '', 'a' => 'From the depths' )); >+ # Add two biblio records >+ my $MARC1 = MARC::Record->new; > $MARC1->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Bruce Wayne', 'b' => '2014', '9' => $authid1 )); >- my $MARC2 = MARC::Record->new(); >- $MARC2->append_fields( MARC::Field->new( '245', '', '', 'a' => 'All the way to heaven' )); >+ $MARC1->append_fields( MARC::Field->new( '245', '', '', 'a' => 'From the depths' )); >+ my $MARC2 = MARC::Record->new; > $MARC2->append_fields( MARC::Field->new( '109', '', '', 'a' => 'Batman', '9' => $authid1 )); >+ $MARC2->append_fields( MARC::Field->new( '245', '', '', 'a' => 'All the way to heaven' )); > my ( $biblionumber1 ) = AddBiblio( $MARC1, ''); > my ( $biblionumber2 ) = AddBiblio( $MARC2, ''); > >+ # Time to merge > @zebrarecords = ( $MARC1, $MARC2 ); > $index = 0; >- > my $rv = C4::AuthoritiesMarc::merge( $authid1, $auth1old, $authid1, $auth1new ); > is( $rv, 2, 'Both records are updated now' ); > >+ #Check the results > my $biblio1 = GetMarcBiblio($biblionumber1); >+ compare_field_count( $MARC1, $biblio1, 1 ); >+ compare_field_order( $MARC1, $biblio1, 1 ); >+ is( $auth1new->field(109)->subfield('a'), $biblio1->field(109)->subfield('a'), 'Record1 values updated correctly' ); > my $biblio2 = GetMarcBiblio($biblionumber1); >- >- my $auth_field = $auth1new->field(109)->subfield('a'); >- is( $auth_field, $biblio1->field(109)->subfield('a'), 'Record1 values updated correctly' ); >- is( $auth_field, $biblio2->field(109)->subfield('a'), 'Record2 values updated correctly' ); >+ compare_field_count( $MARC2, $biblio2, 1 ); >+ compare_field_order( $MARC2, $biblio2, 1 ); >+ is( $auth1new->field(109)->subfield('a'), $biblio2->field(109)->subfield('a'), 'Record2 values updated correctly' ); > > # TODO Following test will change when we improve merge > # Will depend on a preference >@@ -126,42 +127,13 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { > # The merge routine still needs the fixes on bug 17913 > plan tests => 8; > >- # create another authtype >- my $authtype2 = $builder->build({ >- source => 'AuthType', >- value => { >- auth_tag_to_report => '112', >- }, >- }); >- # create two fields linked to this auth type >- $schema->resultset('MarcSubfieldStructure')->search({ tagfield => [ '112', '712' ] })->delete; >- $builder->build({ >- source => 'MarcSubfieldStructure', >- value => { >- tagfield => '112', >- tagsubfield => 'a', >- authtypecode => $authtype2->{authtypecode}, >- frameworkcode => '', >- }, >- }); >- $builder->build({ >- source => 'MarcSubfieldStructure', >- value => { >- tagfield => '712', >- tagsubfield => 'a', >- authtypecode => $authtype2->{authtypecode}, >- frameworkcode => '', >- }, >- }); >- >- # create auth1 (from the earlier type) >+ # create two auth recs of different type > my $auth1 = MARC::Record->new; > $auth1->append_fields( MARC::Field->new( '109', '0', '0', 'a' => 'George Orwell', b => 'bb' )); >- my $authid1 = AddAuthority($auth1, undef, 'TEST_PERSO'); >- # create auth2 (new type) >+ my $authid1 = AddAuthority( $auth1, undef, $authtype1 ); > my $auth2 = MARC::Record->new; > $auth2->append_fields( MARC::Field->new( '112', '0', '0', 'a' => 'Batman', c => 'cc' )); >- my $authid2 = AddAuthority($auth1, undef, $authtype2->{authtypecode} ); >+ my $authid2 = AddAuthority($auth1, undef, $authtype2 ); > > # create a biblio with one 109 and two 609s to be touched > # seems exceptional see bug 13760 comment10 >@@ -178,6 +150,7 @@ subtest 'Test merge A1 to B1 (changing authtype)' => sub { > my ( $biblionumber ) = C4::Biblio::AddBiblio( $marc, '' ); > my $oldbiblio = C4::Biblio::GetMarcBiblio( $biblionumber ); > >+ # Time to merge > @zebrarecords = ( $marc ); > $index = 0; > my $retval = C4::AuthoritiesMarc::merge( $authid1, $auth1, $authid2, $auth2 ); >@@ -225,6 +198,64 @@ sub set_mocks { > $zoom_record_obj->mock( 'raw', sub {} ); > } > >+sub modify_framework { >+ # create two auth types >+ my $authtype1 = $builder->build({ >+ source => 'AuthType', >+ value => { >+ auth_tag_to_report => '109', >+ }, >+ }); >+ my $authtype2 = $builder->build({ >+ source => 'AuthType', >+ value => { >+ auth_tag_to_report => '112', >+ }, >+ }); >+ >+ # Link 109/609 to the first authtype >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ tagfield => '109', >+ tagsubfield => 'a', >+ authtypecode => $authtype1->{authtypecode}, >+ frameworkcode => '', >+ }, >+ }); >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ tagfield => '609', >+ tagsubfield => 'a', >+ authtypecode => $authtype1->{authtypecode}, >+ frameworkcode => '', >+ }, >+ }); >+ >+ # Link 112/712 to the second authtype >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ tagfield => '112', >+ tagsubfield => 'a', >+ authtypecode => $authtype2->{authtypecode}, >+ frameworkcode => '', >+ }, >+ }); >+ $builder->build({ >+ source => 'MarcSubfieldStructure', >+ value => { >+ tagfield => '712', >+ tagsubfield => 'a', >+ authtypecode => $authtype2->{authtypecode}, >+ frameworkcode => '', >+ }, >+ }); >+ >+ return ( $authtype1->{authtypecode}, $authtype2->{authtypecode} ); >+} >+ > sub compare_field_count { > my ( $oldmarc, $newmarc, $pass ) = @_; > my $t; >-- >2.11.0
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 17909
:
59005
|
59006
|
59007
|
59008
|
59011
|
59012
|
59013
|
59014
|
59015
|
59077
|
59088
|
59323
|
59324
|
59334
|
59335
|
59336
| 59337 |
59338
|
59339
|
59340