https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13146
--- Comment #8 from M. Tompsett <[email protected]> --- Comment on attachment 44132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=44132 Bug 13146 UT Review of attachment 44132: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=13146&attachment=44132) ----------------------------------------------------------------- Test::DBIx::Class isn't required to run Koha, just the tests, correct? Probably should skip tests we know we can't run. This will run fine on kohadevbox, but not necessarily on another users koha git box. ::: t/db_dependent/FieldMapping.t @@ +21,5 @@ > +use File::Basename; > +use File::Path; > +use DateTime; > +use Test::MockModule; > +use Test::More tests => 18; Just use Test::More; because of suggested change below. @@ +25,5 @@ > +use Test::More tests => 18; > +use C4::Biblio; > +use Koha::Schema; > + > + use Module::Load::Conditional qw/check_install/; BEGIN { if ( check_install( module => 'Test::DBIx::Class' ) ) { plan tests => 18; # or is it 19... whatever works. } else { plan skip_all => "Need Test::DBIx::Class" } } @@ +43,5 @@ > + _new_schema => sub { return Schema(); } > +); > + > + > +if (0) { What is the point of keeping the code, if it isn't run? @@ +55,5 @@ > +} > + > + > +SetFieldMapping('', 'maintitle', '245', 'a'); > +ok my $fm = Fieldmapping->find( {field => 'maintitle'} ) Multiple side-effects on the same line of code is more difficult to read. @@ +56,5 @@ > + > + > +SetFieldMapping('', 'maintitle', '245', 'a'); > +ok my $fm = Fieldmapping->find( {field => 'maintitle'} ) > + => 'maintitle field mapping properly set for default framework'; While this is valid, given that you did the simpler version of is() below, could this not be: my $fm = Fieldmapping->find( {field => 'maintitle'} ); ok( defined $fm, 'maintitle field mapping properly set for default framework'); instead? This is more readable. @@ +60,5 @@ > + => 'maintitle field mapping properly set for default framework'; > + > +SetFieldMapping('', 'subtitle', '245', 'b'); > +ok $fm = Fieldmapping->find( {field => 'subtitle'} ) > + => 'subtitle field mapping properly set for default framework'; same here. @@ +70,5 @@ > +}, 'expected fields are there'; > + > +SetFieldMapping('BOOK', 'subtitle', '245', 'b'); > +ok $fm = Fieldmapping->find( {frameworkcode => 'BOOK', field => 'subtitle'} ) > + => 'subtitle field mapping properly set for BOOK framework'; same here. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
