From 9de3872b907c6771cf8b70ebc303247d8dddb87c Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 10 Mar 2016 14:42:08 +0100 Subject: [PATCH] Bug 16155: Adjust TestBuilder.t The changes in TestBuilder.pm require some changes in this test. [1] Tests have been organized under subtests. A few superfluous tests have been removed. (There is still some overlap between the sections of overduerules_transport_type and userpermission.) [2] The results in the build all sources-test are checked one step further. [3] Tests are added for field length, null values and delete method. [4] The former defaults from TestBuilder are incorporated in the tests for userpermission. Test plan: Run t/db_dependent/TestBuilder.t Signed-off-by: Bernardo Gonzalez Kriegel Signed-off-by: Kyle M Hall --- t/db_dependent/TestBuilder.t | 481 +++++++++++++++++++---------------- 1 file changed, 262 insertions(+), 219 deletions(-) diff --git a/t/db_dependent/TestBuilder.t b/t/db_dependent/TestBuilder.t index a3cb813be6..0baec3884d 100644 --- a/t/db_dependent/TestBuilder.t +++ b/t/db_dependent/TestBuilder.t @@ -19,7 +19,9 @@ use Modern::Perl; -use Test::More tests => 41; +use Test::More tests => 9; +use Test::Warn; +use Data::Dumper qw(Dumper); use Koha::Database; @@ -27,234 +29,269 @@ BEGIN { use_ok('t::lib::TestBuilder'); } -my $schema = Koha::Database->new->schema; +my $schema = Koha::Database->new->schema; $schema->storage->txn_begin; +my $builder; -my $builder = t::lib::TestBuilder->new(); -is( $builder->build(), undef, 'build without arguments returns undef' ); +subtest 'Start with some trivial tests' => sub { + plan tests => 6; -my @sources = $builder->schema->sources; -my @source_in_failure; -for my $source (@sources) { - eval { $builder->build( { source => $source } ); }; - push @source_in_failure, $source if $@; -} -is( @source_in_failure, 0, 'TestBuilder should be able to create an object for every sources' ); -if ( @source_in_failure ) { - diag ("The following sources have not been generated correctly: " . join ', ', @source_in_failure) -} + $builder = t::lib::TestBuilder->new; + isnt( $builder, undef, 'We got a builder' ); + + is( $builder->build, undef, 'build without arguments returns undef' ); + is( ref( $builder->schema ), 'Koha::Schema', 'check schema' ); + is( ref( $builder->can('delete') ), 'CODE', 'found delete method' ); -my $my_overduerules_transport_type = { - message_transport_type => { - message_transport_type => 'my msg_t_t', - }, - overduerules_id => { - branchcode => 'codeB', - categorycode => 'codeC', - }, - categorycode => undef, + # invalid argument + warning_like { $builder->build({ + source => 'Borrower', + value => { surname => { invalid_hash => 1 } }, + }) } qr/^Hash not allowed for surname/, + 'Build should not accept a hash for this column'; + + # return undef if a record exists + my $param = { source => 'Branch', value => { branchcode => 'MPL' } }; + $builder->build( $param ); # at least it should exist now + is( $builder->build( $param ), undef, 'Return undef when exists' ); }; -$my_overduerules_transport_type->{categorycode} = $my_overduerules_transport_type->{branchcode}; -my $overduerules_transport_type = $builder->build({ - source => 'OverduerulesTransportType', - value => $my_overduerules_transport_type, -}); -is( - $overduerules_transport_type->{message_transport_type}, - $my_overduerules_transport_type->{message_transport_type}->{message_transport_type}, - 'build stores the message_transport_type correctly' -); -is( - $overduerules_transport_type->{branchcode}, - $my_overduerules_transport_type->{branchcode}->{branchcode}, - 'build stores the branchcode correctly' -); -is( - $overduerules_transport_type->{categorycode}, - $my_overduerules_transport_type->{categorycode}->{categorycode}, - 'build stores the categorycode correctly' -); -is( - $overduerules_transport_type->{_fk}->{message_transport_type}->{message_transport_type}, - $my_overduerules_transport_type->{message_transport_type}->{message_transport_type}, - 'build stores the foreign key message_transport_type correctly' -); -is( - $overduerules_transport_type->{_fk}->{branchcode}->{branchcode}, - $my_overduerules_transport_type->{branchcode}->{branchcode}, - 'build stores the foreign key branchcode correctly' -); -is( - $overduerules_transport_type->{_fk}->{categorycode}->{categorycode}, - $my_overduerules_transport_type->{categorycode}->{categorycode}, - 'build stores the foreign key categorycode correctly' -); -is_deeply( - $overduerules_transport_type->{_fk}->{branchcode}, - $overduerules_transport_type->{_fk}->{categorycode}, - 'build links the branchcode and the categorycode correctly' -); -isnt( - $overduerules_transport_type->{_fk}->{overduerules_id}->{letter2}, - undef, - 'build generates values if they are not given' -); - -my $my_user_permission = $t::lib::TestBuilder::default_value->{UserPermission}; -my $user_permission = $builder->build({ - source => 'UserPermission', -}); -isnt( - $user_permission->{borrowernumber}, - undef, - 'build generates a borrowernumber correctly' -); -is( - $user_permission->{module_bit}, - $my_user_permission->{module_bit}->{module_bit}->{bit}, - 'build stores the default value correctly' -); -is( - $user_permission->{code}, - $my_user_permission->{module_bit}->{code}, - 'build stores the default value correctly' -); -is( - $user_permission->{borrowernumber}, - $user_permission->{_fk}->{borrowernumber}->{borrowernumber}, - 'build links the foreign key correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{surname}, - $my_user_permission->{borrowernumber}->{surname}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{address}, - $my_user_permission->{borrowernumber}->{address}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{city}, - $my_user_permission->{borrowernumber}->{city}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{branchcode}->{branchcode}, - $my_user_permission->{borrowernumber}->{branchcode}->{branchcode}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{branchcode}->{branchname}, - $my_user_permission->{borrowernumber}->{branchcode}->{branchname}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{categorycode}->{categorycode}, - $my_user_permission->{borrowernumber}->{categorycode}->{categorycode}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{categorycode}->{hidelostitems}, - $my_user_permission->{borrowernumber}->{categorycode}->{hidelostitems}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{categorycode}->{category_type}, - $my_user_permission->{borrowernumber}->{categorycode}->{category_type}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{categorycode}->{defaultprivacy}, - $my_user_permission->{borrowernumber}->{categorycode}->{defaultprivacy}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{borrowernumber}->{privacy}, - $my_user_permission->{borrowernumber}->{privacy}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{module_bit}->{_fk}->{module_bit}->{bit}, - $my_user_permission->{module_bit}->{module_bit}->{bit}, - 'build stores the foreign key value correctly' -); -is( - $user_permission->{_fk}->{module_bit}->{code}, - $my_user_permission->{module_bit}->{code}, - 'build stores the foreign key value correctly' -); -is_deeply( - $user_permission->{_fk}->{module_bit}, - $user_permission->{_fk}->{code}, - 'build links the codes correctly' -); -isnt( - $user_permission->{_fk}->{borrowernumber}->{cardnumber}, - undef, - 'build generates values if they are not given' -); -isnt( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{branchcode}->{branchaddress1}, - undef, - 'build generates values if they are not given' -); -isnt( - $user_permission->{_fk}->{borrowernumber}->{_fk}->{categorycode}->{description}, - undef, - 'build generates values if they are not given' -); -isnt( - $user_permission->{_fk}->{module_bit}->{description}, - undef, - 'build generates values if they are not given' -); -isnt( - $user_permission->{_fk}->{module_bit}->{_fk}->{module_bit}->{flag}, - undef, - 'build generates values if they are not given' -); - - -my $nb_basket = $builder->schema->resultset('Aqbasket')->search(); -isnt( $nb_basket, 0, 'add stores the generated entries correctly' ); -$builder->clear( { source => 'Aqbasket' } ); -$nb_basket = $builder->schema->resultset('Aqbasket')->search(); -is( $nb_basket, 0, 'clear removes all the entries correctly' ); - - -my $rs_aqbookseller = $builder->schema->resultset('Aqbookseller'); -my $bookseller = $builder->build({ - source => 'Aqbookseller', - only_fk => 1, -}); -delete $bookseller->{_fk}; -my $bookseller_from_db = $rs_aqbookseller->find($bookseller); -is( $bookseller_from_db, undef, 'build with only_fk = 1 does not store the entry' ); -my $bookseller_result = $rs_aqbookseller->create($bookseller); -is( $bookseller_result->in_storage, 1, 'build with only_fk = 1 creates the foreign keys correctly' ); - -$bookseller = $builder->build({ - source => 'Aqbookseller', -}); -ok( length( $bookseller->{phone} ) <= 30, 'The length for a generated string should not be longer than the size of the DB field' ); -delete $bookseller->{_fk}; -$bookseller_from_db = $rs_aqbookseller->find($bookseller); -is( $bookseller_from_db->in_storage, 1, 'build without the parameter only_sk stores the entry correctly' ); - -$bookseller = $builder->build({ - source => 'Aqbookseller', - only_fk => 0, -}); -delete $bookseller->{_fk}; -$bookseller_from_db = $rs_aqbookseller->find($bookseller); -is( $bookseller_from_db->in_storage, 1, 'build with only_fk = 0 stores the entry correctly' ); -subtest 'Auto-increment values tests' => sub { +subtest 'Build all sources' => sub { + plan tests => 1; + + my @sources = $builder->schema->sources; + my @source_in_failure; + for my $source ( @sources ) { + my $res; + eval { $res = $builder->build( { source => $source } ); }; + push @source_in_failure, $source if $@ || !defined( $res ); + } + is( @source_in_failure, 0, + 'TestBuilder should be able to create an object for every source' ); + if ( @source_in_failure ) { + diag( "The following sources have not been generated correctly: " . + join ', ', @source_in_failure ); + } +}; + + +subtest 'Test length of some generated fields' => sub { plan tests => 2; + # Test the length of a returned character field + my $bookseller = $builder->build({ source => 'Aqbookseller' }); + my $max = $schema->source('Aqbookseller')->column_info('phone')->{size}; + is( length( $bookseller->{phone} ) > 0, 1, + 'The length for a generated string (phone) should not be zero' ); + is( length( $bookseller->{phone} ) <= $max, 1, + 'Check maximum length for a generated string (phone)' ); +}; + + +subtest 'Test FKs in overduerules_transport_type' => sub { + plan tests => 5; + + my $my_overduerules_transport_type = { + message_transport_type => { + message_transport_type => 'my msg_t_t', + }, + overduerules_id => { + branchcode => 'codeB', + categorycode => 'codeC', + }, + }; + + my $overduerules_transport_type = $builder->build({ + source => 'OverduerulesTransportType', + value => $my_overduerules_transport_type, + }); + is( + $overduerules_transport_type->{message_transport_type}, + $my_overduerules_transport_type->{message_transport_type}->{message_transport_type}, + 'build stores the message_transport_type correctly' + ); + is( + $schema->resultset('Overduerule')->find( $overduerules_transport_type->{overduerules_id} )->branchcode, + $my_overduerules_transport_type->{overduerules_id}->{branchcode}, + 'build stores the branchcode correctly' + ); + is( + $schema->resultset('Overduerule')->find( $overduerules_transport_type->{overduerules_id} )->categorycode, + $my_overduerules_transport_type->{overduerules_id}->{categorycode}, + 'build stores the categorycode correctly' + ); + is( + $schema->resultset('MessageTransportType')->find( $overduerules_transport_type->{message_transport_type} )->message_transport_type, + $overduerules_transport_type->{message_transport_type}, + 'build stores the foreign key message_transport_type correctly' + ); + isnt( + $schema->resultset('Overduerule')->find( $my_overduerules_transport_type->{overduerules_id} )->letter2, + undef, + 'build generates values if they are not given' + ); +}; + + +subtest 'Tests with composite FK in userpermission' => sub { + plan tests => 9; + + my $my_user_permission = default_userpermission(); + my $user_permission = $builder->build({ + source => 'UserPermission', + value => $my_user_permission, + }); + + # Checks on top level of userpermission + isnt( + $user_permission->{borrowernumber}, + undef, + 'build generates a borrowernumber correctly' + ); + is( + $user_permission->{code}, + $my_user_permission->{code}->{code}, + 'build stores code correctly' + ); + + # Checks one level deeper userpermission -> borrower + my $patron = $schema->resultset('Borrower')->find({ borrowernumber => $user_permission->{borrowernumber} }); + is( + $patron->surname, + $my_user_permission->{borrowernumber}->{surname}, + 'build stores surname correctly' + ); + isnt( + $patron->cardnumber, + undef, + 'build generated cardnumber' + ); + + # Checks two levels deeper userpermission -> borrower -> branch + my $branch = $schema->resultset('Branch')->find({ branchcode => $patron->branchcode->branchcode }); + is( + $branch->branchname, + $my_user_permission->{borrowernumber}->{branchcode}->{branchname}, + 'build stores branchname correctly' + ); + isnt( + $branch->branchaddress1, + undef, + 'build generated branch address' + ); + + # Checks with composite FK: userpermission -> permission + my $perm = $schema->resultset('Permission')->find({ module_bit => $user_permission->{module_bit}, code => $my_user_permission->{code}->{code} }); + isnt( $perm, undef, 'build generated record for composite FK' ); + is( + $perm->code, + $my_user_permission->{code}->{code}, + 'build stored code correctly' + ); + is( + $perm->description, + $my_user_permission->{code}->{description}, + 'build stored description correctly' + ); +}; + +sub default_userpermission { + return { + borrowernumber => { + surname => 'my surname', + address => 'my adress', + city => 'my city', + branchcode => { + branchname => 'my branchname', + }, + categorycode => { + hidelostitems => 0, + category_type => 'A', + default_privacy => 'default', + }, + privacy => 1, + }, + module_bit => { + flag => 'my flag', + }, + code => { + code => 'my code', + description => 'my desc', + }, + }; +} + + +subtest 'Test build with NULL values' => sub { + plan tests => 3; + + # PK should not be null + my $params = { source => 'Branch', value => { branchcode => undef }}; + is( $builder->build( $params ), undef, 'PK should not be null' ); + # Nullable column + my $info = $schema->source( 'Item' )->column_info( 'barcode' ); + $params = { source => 'Item', value => { barcode => undef }}; + my $item = $builder->build( $params ); + is( $info->{is_nullable} && $item && !defined( $item->{barcode} ), 1, + 'Barcode can be NULL' ); + # Nullable FK + $params = { source => 'Reserve', value => { itemnumber => undef }}; + my $reserve = $builder->build( $params ); + $info = $schema->source( 'Reserve' )->column_info( 'itemnumber' ); + is( $reserve && $info->{is_nullable} && $info->{is_foreign_key} && + !defined( $reserve->{itemnumber} ), 1, 'Nullable FK' ); +}; + + +subtest 'Tests for delete method' => sub { + plan tests => 12; + + # Test delete with single and multiple records + my $basket1 = $builder->build({ source => 'Aqbasket' }); + my $basket2 = $builder->build({ source => 'Aqbasket' }); + my $basket3 = $builder->build({ source => 'Aqbasket' }); + my ( $id1, $id2 ) = ( $basket1->{basketno}, $basket2->{basketno} ); + $builder->delete({ source => 'Aqbasket', records => $basket1 }); + isnt( exists $basket1->{basketno}, 1, 'Delete cleared PK hash value' ); + + is( $builder->schema->resultset('Aqbasket')->search({ basketno => $id1 })->count, 0, 'Basket1 is no longer found' ); + is( $builder->schema->resultset('Aqbasket')->search({ basketno => $id2 })->count, 1, 'Basket2 is still found' ); + is( $builder->delete({ source => 'Aqbasket', records => [ $basket2, $basket3 ] }), 2, "Returned two delete attempts" ); + is( $builder->schema->resultset('Aqbasket')->search({ basketno => $id2 })->count, 0, 'Basket2 is no longer found' ); + + + # Test delete in table without primary key (..) + is( $schema->source('TmpHoldsqueue')->primary_columns, 0, + 'Table without primary key detected' ); + my $bibno = 123; # just a number + my $cnt1 = $schema->resultset('TmpHoldsqueue')->count; + # Insert a new record in TmpHoldsqueue with that biblionumber + my $val = { biblionumber => $bibno }; + my $rec = $builder->build({ source => 'TmpHoldsqueue', value => $val }); + my $cnt2 = $schema->resultset('TmpHoldsqueue')->count; + is( defined($rec) && $cnt2 == $cnt1 + 1 , 1, 'Created a record' ); + is( $builder->delete({ source => 'TmpHoldsqueue', records => $rec }), + undef, 'delete returns undef' ); + is( $rec->{biblionumber}, $bibno, 'Hash value untouched' ); + is( $schema->resultset('TmpHoldsqueue')->count, $cnt2, + "Method did not delete record in table without PK" ); + + # Test delete with NULL values + $val = { branchcode => undef }; + is( $builder->delete({ source => 'Branch', records => $val }), 0, + 'delete returns zero for an undef search with one key' ); + $val = { module_bit => 1, #catalogue + code => undef }; + is( $builder->delete({ source => 'Permission', records => $val }), 0, + 'delete returns zero for an undef search with a composite PK' ); +}; + + +subtest 'Auto-increment values tests' => sub { + plan tests => 3; + # Pick a table with AI PK my $source = 'Biblio'; # table my $column = 'biblionumber'; # ai column @@ -272,6 +309,12 @@ subtest 'Auto-increment values tests' => sub { my $next_ai_value = $biblio_2->{ biblionumber }; is( $ai_value + 1, $next_ai_value, "AI values are consecutive"); + # respect autoincr column + warning_like { $builder->build({ + source => $source, + value => { biblionumber => 123 }, + }) } qr/^Value not allowed for auto_incr/, + 'Build should not overwrite an auto_incr column'; }; $schema->storage->txn_rollback; -- 2.39.5