From 28055d334741f8045423f9ac54106050e6b40d01 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 6 Aug 2019 14:38:28 +0100 Subject: [PATCH] Bug 23382: Improve test coverage Signed-off-by: Jonathan Druart Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- Koha/Charges/Fees.pm | 30 +++- t/db_dependent/Koha/Charges/Fees.t | 260 ++++++++++++++++++++++++++--- 2 files changed, 264 insertions(+), 26 deletions(-) diff --git a/Koha/Charges/Fees.pm b/Koha/Charges/Fees.pm index 41299cee14..96a9897ac1 100644 --- a/Koha/Charges/Fees.pm +++ b/Koha/Charges/Fees.pm @@ -19,7 +19,7 @@ package Koha::Charges::Fees; use Modern::Perl; -use Carp qw( confess ); +use Carp qw( carp confess ); use Koha::Calendar; use Koha::IssuingRules; @@ -82,7 +82,7 @@ sub new { my $fee = $self->accumulate_rentalcharge(); - This method calculates the daily rental fee for a given itemtype for a given + This method calculates the daily/hourly rental fee for a given itemtype for a given period of time passed in as a pair of DateTime objects. =cut @@ -138,7 +138,10 @@ my $patron = $fees->patron( $patron ); sub patron { my ( $self, $patron ) = @_; - $self->{patron} = $patron if $patron && $patron->isa('Koha::Patron'); + Carp::carp("Setting 'patron' to something other than a Koha::Patron is not supported!") + if ($patron && !$patron->isa('Koha::Patron')); + + $self->{patron} = $patron if $patron; return $self->{patron}; } @@ -152,7 +155,10 @@ my $library = $fees->library( $library ); sub library { my ( $self, $library ) = @_; - $self->{library} = $library if $library && $library->isa('Koha::Library'); + Carp::carp("Setting 'library' to something other than a Koha::Library is not supported!") + if ($library && !$library->isa('Koha::Library')); + + $self->{library} = $library if $library; return $self->{library}; } @@ -166,7 +172,10 @@ my $item = $fees->item( $item ); sub item { my ( $self, $item ) = @_; - $self->{item} = $item if $item && $item->isa('Koha::Item'); + Carp::carp("Setting 'item' to something other than a Koha::Item is not supported!") + if ($item && !$item->isa('Koha::Item')); + + $self->{item} = $item if $item; return $self->{item}; } @@ -180,7 +189,10 @@ my $to_date = $fees->to_date( $to_date ); sub to_date { my ( $self, $to_date ) = @_; - $self->{to_date} = $to_date if $to_date && $to_date->isa('DateTime'); + Carp::carp("Setting 'to_date' to something other than a DateTime is not supported!") + if ($to_date && !$to_date->isa('DateTime')); + + $self->{to_date} = $to_date if $to_date; return $self->{to_date}; } @@ -194,14 +206,18 @@ my $from_date = $fees->from_date( $from_date ); sub from_date { my ( $self, $from_date ) = @_; + Carp::carp("Setting 'from_date' to something other than a DateTime is not supported!") + if ($from_date && !$from_date->isa('DateTime')); + $self->{from_date} = $from_date if $from_date && $from_date->isa('DateTime'); return $self->{from_date}; } -=head1 AUTHOR +=head1 AUTHORS Kyle M Hall +Martin Renvoize =cut diff --git a/t/db_dependent/Koha/Charges/Fees.t b/t/db_dependent/Koha/Charges/Fees.t index cc555779b5..1722fc191c 100644 --- a/t/db_dependent/Koha/Charges/Fees.t +++ b/t/db_dependent/Koha/Charges/Fees.t @@ -19,13 +19,14 @@ use Modern::Perl; -use Test::More tests => 2; +use Test::More tests => 8; +use Test::Exception; +use Test::Warn; use t::lib::Mocks; use t::lib::TestBuilder; -use Data::Dumper; - +use Time::Fake; use C4::Calendar; use Koha::DateUtils qw(dt_from_string); @@ -59,9 +60,9 @@ my $itemtype = $builder->build_object( class => 'Koha::ItemTypes', value => { rentalcharge_daily => '0.00', - rentalcharge => '0.00', - processfee => '0.00', - defaultreplacecost => '0.00', + rentalcharge => '0.00', + processfee => '0.00', + defaultreplacecost => '0.00', }, } ); @@ -86,22 +87,235 @@ my $patron = $builder->build_object( } ); -my $dt_from = dt_from_string(); -my $dt_to = dt_from_string()->add( days => 6 ); +my $dt = dt_from_string(); +Time::Fake->offset( $dt->epoch ); -my $fees = Koha::Charges::Fees->new( - { - patron => $patron, - library => $library, - item => $item, - to_date => $dt_to, - from_date => $dt_from, +my $dt_from = dt_from_string()->subtract( days => 2 ); +my $dt_to = dt_from_string()->add( days => 4 ); + +subtest 'new' => sub { + plan tests => 9; + + # Mandatory parameters missing + throws_ok { + Koha::Charges::Fees->new( + { + library => $library, + item => $item, + to_date => $dt_to, + } + ) } -); + 'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for patron'; + throws_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + item => $item, + to_date => $dt_to, + } + ) + } + 'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for library'; + throws_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + to_date => $dt_to, + } + ) + } + 'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for item'; + throws_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + } + ) + } + 'Koha::Exceptions::MissingParameter', 'MissingParameter thrown for to_date'; + + # Mandatory parameter bad + dies_ok { + Koha::Charges::Fees->new( + { + patron => '12345', + library => $library, + item => $item, + to_date => $dt_to, + } + ) + } + 'dies for bad patron'; + dies_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + library => '12345', + item => $item, + to_date => $dt_to, + } + ) + } + 'dies for bad library'; + dies_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => '12345', + to_date => $dt_to, + } + ) + } + 'dies for bad item'; + dies_ok { + Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => 12345 + } + ) + } + 'dies for bad to_date'; + + # Defaults + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + is( $fees->from_date, dt_from_string(), + 'from_date default set correctly to today' ); +}; + +subtest 'patron accessor' => sub { + plan tests => 2; + + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + + ok( + $fees->patron->isa('Koha::Patron'), + 'patron accessor returns a Koha::Patron' + ); + warning_is { $fees->patron('12345') } + { carped => +"Setting 'patron' to something other than a Koha::Patron is not supported!" + }, "Warning thrown when attempting to set patron to string"; + +}; + +subtest 'library accessor' => sub { + plan tests => 2; + + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + + ok( + $fees->library->isa('Koha::Library'), + 'library accessor returns a Koha::Library' + ); + warning_is { $fees->library('12345') } + { carped => +"Setting 'library' to something other than a Koha::Library is not supported!" + }, "Warning thrown when attempting to set library to string"; +}; + +subtest 'item accessor' => sub { + plan tests => 2; + + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + + ok( $fees->item->isa('Koha::Item'), 'item accessor returns a Koha::Item' ); + warning_is { $fees->item('12345') } + { carped => +"Setting 'item' to something other than a Koha::Item is not supported!" + }, "Warning thrown when attempting to set item to string"; +}; + +subtest 'to_date accessor' => sub { + plan tests => 2; + + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + + ok( $fees->to_date->isa('DateTime'), + 'to_date accessor returns a DateTime' ); + warning_is { $fees->to_date(12345) } + { carped => +"Setting 'to_date' to something other than a DateTime is not supported!" + }, "Warning thrown when attempting to set to_date to integer"; +}; + +subtest 'from_date accessor' => sub { + plan tests => 2; + + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + } + ); + + ok( + $fees->from_date->isa('DateTime'), + 'from_date accessor returns a DateTime' + ); + warning_is { $fees->from_date(12345) } + { carped => +"Setting 'from_date' to something other than a DateTime is not supported!" + }, "Warning thrown when attempting to set from_date to integer"; +}; subtest 'accumulate_rentalcharge tests' => sub { plan tests => 4; + my $fees = Koha::Charges::Fees->new( + { + patron => $patron, + library => $library, + item => $item, + to_date => $dt_to, + from_date => $dt_from, + } + ); + $itemtype->rentalcharge_daily(1.00); $itemtype->store(); is( $itemtype->rentalcharge_daily, @@ -109,11 +323,15 @@ subtest 'accumulate_rentalcharge tests' => sub { t::lib::Mocks::mock_preference( 'finesCalendar', 'ignoreCalendar' ); my $charge = $fees->accumulate_rentalcharge(); - is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' ); + is( $charge, 6.00, +'Daily rental charge calculated correctly with finesCalendar = ignoreCalendar' + ); t::lib::Mocks::mock_preference( 'finesCalendar', 'noFinesWhenClosed' ); $charge = $fees->accumulate_rentalcharge(); - is( $charge, 6.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' ); + is( $charge, 6.00, +'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed' + ); my $calendar = C4::Calendar->new( branchcode => $library->id ); $calendar->insert_week_day_holiday( @@ -122,5 +340,9 @@ subtest 'accumulate_rentalcharge tests' => sub { description => 'Test holiday' ); $charge = $fees->accumulate_rentalcharge(); - is( $charge, 5.00, 'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' ); + is( $charge, 5.00, +'Daily rental charge calculated correctly with finesCalendar = noFinesWhenClosed and closed Wednesdays' + ); }; + +Time::Fake->reset; -- 2.39.5