From 99410d7ce47a1fd4af1bcd534d0135b62b52c2e5 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Tue, 4 Dec 2012 15:14:59 -0300 Subject: [PATCH] Bug 9209 - Mocked Koha::Calendar tests MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Using specific method for populating the internal data structures from Koha::Calendar has yielded to the non-detection of several bugs. There are also several tests that where db_dependent which is not always desirable. I propose the use of DBD::Mock (::Session) for using the actual code used by Koha in production for testing, mocking the DB queries itselves. I also took the time to repeat several tests in different syspref configurations (they applied only to daysMode=Calendar, and now cover all confs). Notes: - I used DBD:Mock 1.45 as previous version (1.43, from 12.04) was broken - Some tests revealed a bug on days_between as I see it... reporting as Bug #9211 Sponsored-by: Universidad Nacional de Córdoba Signed-off-by: Elliott Davis Signed-off-by: Jared Camins-Esakov --- Koha/Calendar.pm | 30 ++-- t/Calendar.t | 351 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 264 insertions(+), 117 deletions(-) diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 90069b6a89..45841beeb9 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -33,28 +33,31 @@ sub _init { my $self = shift; my $branch = $self->{branchcode}; my $dbh = C4::Context->dbh(); - my $repeat_sth = $dbh->prepare( -'SELECT * from repeatable_holidays WHERE branchcode = ? AND ISNULL(weekday) = ?' + my $weekly_closed_days_sth = $dbh->prepare( +'SELECT weekday FROM repeatable_holidays WHERE branchcode = ? AND weekday IS NOT NULL' ); - $repeat_sth->execute( $branch, 0 ); + $weekly_closed_days_sth->execute( $branch ); $self->{weekly_closed_days} = [ 0, 0, 0, 0, 0, 0, 0 ]; Readonly::Scalar my $sunday => 7; - while ( my $tuple = $repeat_sth->fetchrow_hashref ) { + while ( my $tuple = $weekly_closed_days_sth->fetchrow_hashref ) { $self->{weekly_closed_days}->[ $tuple->{weekday} ] = 1; } - $repeat_sth->execute( $branch, 1 ); + my $day_month_closed_days_sth = $dbh->prepare( +'SELECT day, month FROM repeatable_holidays WHERE branchcode = ? AND weekday IS NULL' + ); + $day_month_closed_days_sth->execute( $branch ); $self->{day_month_closed_days} = {}; - while ( my $tuple = $repeat_sth->fetchrow_hashref ) { + while ( my $tuple = $day_month_closed_days_sth->fetchrow_hashref ) { $self->{day_month_closed_days}->{ $tuple->{month} }->{ $tuple->{day} } = 1; } - my $special = $dbh->prepare( -'SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = ?' + my $exception_holidays_sth = $dbh->prepare( +'SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 1' ); - $special->execute( $branch, 1 ); + $exception_holidays_sth->execute( $branch ); my $dates = []; - while ( my ( $day, $month, $year ) = $special->fetchrow ) { + while ( my ( $day, $month, $year ) = $exception_holidays_sth->fetchrow ) { push @{$dates}, DateTime->new( day => $day, @@ -66,9 +69,12 @@ sub _init { $self->{exception_holidays} = DateTime::Set->from_datetimes( dates => $dates ); - $special->execute( $branch, 0 ); + my $single_holidays_sth = $dbh->prepare( +'SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 0' + ); + $single_holidays_sth->execute( $branch ); $dates = []; - while ( my ( $day, $month, $year ) = $special->fetchrow ) { + while ( my ( $day, $month, $year ) = $single_holidays_sth->fetchrow ) { push @{$dates}, DateTime->new( day => $day, diff --git a/t/Calendar.t b/t/Calendar.t index 42b8342973..2557ed6a9f 100755 --- a/t/Calendar.t +++ b/t/Calendar.t @@ -4,7 +4,9 @@ use strict; use warnings; use DateTime; use DateTime::Duration; -use Test::More tests => 26; +use Test::More tests => 35; +use Test::MockModule; +use DBD::Mock; use Koha::DateUtils; BEGIN { @@ -15,124 +17,202 @@ BEGIN { use_ok('C4::Calendar'); } -my $cal = Koha::Calendar->new( TEST_MODE => 1 ); +my $module_context = new Test::MockModule('C4::Context'); +$module_context->mock( + '_new_dbh', + sub { + my $dbh = DBI->connect( 'DBI:Mock:', '', '' ) + || die "Cannot create handle: $DBI::errstr\n"; + return $dbh; + } +); + +# We need to mock the C4::Context->preference method for +# simplicity and re-usability of the session definition. Any +# syspref fits for syspref-agnostic tests. +$module_context->mock( + 'preference', + sub { + return 'Calendar'; + } +); + + +my $holidays_session = DBD::Mock::Session->new('holidays_session' => ( + { # weekly holidays + statement => "SELECT weekday FROM repeatable_holidays WHERE branchcode = ? AND weekday IS NOT NULL", + results => [ + ['weekday'], + [0], # sundays + [6] # saturdays + ] + }, + { # day and month repeatable holidays + statement => "SELECT day, month FROM repeatable_holidays WHERE branchcode = ? AND weekday IS NULL", + results => [ + [ 'month', 'day' ], + [ 1, 1 ], # new year's day + [12,25] # christmas + ] + }, + { # exception holidays + statement => "SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 1", + results => [ + [ 'day', 'month', 'year' ], + [ 11, 11, 2012 ] # sunday exception + ] + }, + { # single holidays + statement => "SELECT day, month, year FROM special_holidays WHERE branchcode = ? AND isexception = 0", + results => [ + [ 'day', 'month', 'year' ], + [ 1, 6, 2011 ], # single holiday + [ 4, 7, 2012 ] + ] + } +)); + +# Initialize the global $dbh variable +my $dbh = C4::Context->dbh(); +# Apply the mock session +$dbh->{ mock_session } = $holidays_session; +# 'MPL' branch is arbitrary, is not used at all but is needed for initialization +my $cal = Koha::Calendar->new( branchcode => 'MPL' ); isa_ok( $cal, 'Koha::Calendar', 'Calendar class returned' ); + my $saturday = DateTime->new( - year => 2011, - month => 6, - day => 25, - time_zone => 'Europe/London', + year => 2012, + month => 11, + day => 24, ); + my $sunday = DateTime->new( - year => 2011, - month => 6, - day => 26, - time_zone => 'Europe/London', + year => 2012, + month => 11, + day => 25, ); + my $monday = DateTime->new( - year => 2011, - month => 6, - day => 27, - time_zone => 'Europe/London', + year => 2012, + month => 11, + day => 26, ); -my $bloomsday = DateTime->new( - year => 2011, - month => 6, - day => 16, - time_zone => 'Europe/London', -); # should be a holiday -my $special = DateTime->new( + +my $new_year = DateTime->new( + year => 2013, + month => 1, + day => 1, +); + +my $single_holiday = DateTime->new( year => 2011, month => 6, day => 1, - time_zone => 'Europe/London', ); # should be a holiday + my $notspecial = DateTime->new( year => 2011, month => 6, - day => 2, - time_zone => 'Europe/London', + day => 2 ); # should NOT be a holiday -is( $cal->is_holiday($sunday), 1, 'Sunday is a closed day' ); # wee free test; -is( $cal->is_holiday($monday), 0, 'Monday is not a closed day' ); # alas -is( $cal->is_holiday($bloomsday), 1, 'month/day closed day test' ); -is( $cal->is_holiday($special), 1, 'special closed day test' ); -is( $cal->is_holiday($notspecial), 0, 'open day test' ); +my $sunday_exception = DateTime->new( + year => 2012, + month => 11, + day => 11 +); -my $dt = $cal->addDate( $saturday, 1, 'days' ); -is( $dt->day_of_week, 1, 'addDate skips closed Sunday' ); +my $day_after_christmas = DateTime->new( + year => 2012, + month => 12, + day => 26 +); # for testing negative addDate + + +{ # Syspref-agnostic tests + is ( $saturday->day_of_week, 6, '\'$saturday\' is actually a saturday (6th day of week)'); + is ( $sunday->day_of_week, 7, '\'$sunday\' is actually a sunday (7th day of week)'); + is ( $monday->day_of_week, 1, '\'$monday\' is actually a monday (1st day of week)'); + is ( $cal->is_holiday($saturday), 1, 'Saturday is a closed day' ); + is ( $cal->is_holiday($sunday), 1, 'Sunday is a closed day' ); + is ( $cal->is_holiday($monday), 0, 'Monday is not a closed day' ); + is ( $cal->is_holiday($new_year), 1, 'Month/Day closed day test (New year\'s day)' ); + is ( $cal->is_holiday($single_holiday), 1, 'Single holiday closed day test' ); + is ( $cal->is_holiday($notspecial), 0, 'Fixed single date that is not a holiday test' ); + is ( $cal->is_holiday($sunday_exception), 0, 'Exception holiday is not a closed day test' ); +} -$dt = $cal->addDate( $bloomsday, -1 ); -is( $dt->ymd(), '2011-06-15', 'Negative call to addDate' ); -my $test_dt = DateTime->new( # Monday - year => 2012, - month => 7, - day => 23, - hour => 11, - minute => 53, - time_zone => 'Europe/London', -); +{ # Bugzilla #8966 - is_holiday truncates referenced date + my $later_dt = DateTime->new( # Monday + year => 2012, + month => 9, + day => 17, + hour => 17, + minute => 30, + time_zone => 'Europe/London', + ); -my $later_dt = DateTime->new( # Monday - year => 2012, - month => 9, - day => 17, - hour => 17, - minute => 30, - time_zone => 'Europe/London', -); -my $daycount = $cal->days_between( $test_dt, $later_dt ); -cmp_ok( $daycount->in_units('days'), - '==', 48, 'days_between calculates correctly' ); - -my $ret; - -$cal->set_daysmode('Calendar'); - -# see bugzilla #8966 -is( $cal->is_holiday($later_dt), 0, 'is holiday for the next test' ); -cmp_ok( $later_dt, 'eq', '2012-09-17T17:30:00', 'Date should be the same after is_holiday' ); - -# example tests for bug report -$cal->clear_weekly_closed_days(); - -$daycount = $cal->days_between( dt_from_string('2012-01-10','iso'), - dt_from_string("2012-05-05",'iso') )->in_units('days'); -cmp_ok( $daycount, '==', 116, 'test larger intervals' ); -$daycount = $cal->days_between( dt_from_string("2012-01-01",'iso'), - dt_from_string("2012-05-05",'iso') )->in_units('days'); -cmp_ok( $daycount, '==', 125, 'test positive intervals' ); -my $daycount2 = $cal->days_between( dt_from_string("2012-05-05",'iso'), - dt_from_string("2012-01-01",'iso') )->in_units('days'); -cmp_ok( $daycount2, '==', $daycount, 'test parameter order not relevant' ); -$daycount = $cal->days_between( dt_from_string("2012-07-01",'iso'), - dt_from_string("2012-07-15",'iso') )->in_units('days'); -cmp_ok( $daycount, '==', 14, 'days_between calculates correctly' ); -$cal->add_holiday( dt_from_string('2012-07-06','iso') ); -$daycount = $cal->days_between( dt_from_string("2012-07-01",'iso'), - dt_from_string("2012-07-15",'iso') )->in_units('days'); -cmp_ok( $daycount, '==', 13, 'holiday correctly recognized' ); - -$cal->add_holiday( dt_from_string('2012-07-07','iso') ); -$daycount = $cal->days_between( dt_from_string("2012-07-01",'iso'), - dt_from_string("2012-07-15",'iso') )->in_units('days'); -cmp_ok( $daycount, '==', 12, 'multiple holidays correctly recognized' ); - -my $one_day_dur = DateTime::Duration->new( days => 1 ); -my $two_day_dur = DateTime::Duration->new( days => 2 ); -my $seven_day_dur = DateTime::Duration->new( days => 7 ); - - ## 'Datedue' tests - $cal = Koha::Calendar->new( TEST_MODE => 1 , - days_mode => 'Datedue'); - - $cal->add_holiday( dt_from_string('2012-07-04','iso') ); - $dt = dt_from_string( '2012-07-03','iso' ); + is( $cal->is_holiday($later_dt), 0, 'bz-8966 (1/2) Apply is_holiday for the next test' ); + cmp_ok( $later_dt, 'eq', '2012-09-17T17:30:00', 'bz-8966 (2/2) Date should be the same after is_holiday' ); +} + + +{ # Bugzilla #8800 - is_holiday should use truncated date for 'contains' call + my $single_holiday_time = DateTime->new( + year => 2011, + month => 6, + day => 1, + hour => 11, + minute => 2 + ); + + is( $cal->is_holiday($single_holiday_time), + $cal->is_holiday($single_holiday) , + 'bz-8800 is_holiday should truncate the date for holiday validation' ); +} + + + my $one_day_dur = DateTime::Duration->new( days => 1 ); + my $two_day_dur = DateTime::Duration->new( days => 2 ); + my $seven_day_dur = DateTime::Duration->new( days => 7 ); + + my $dt = dt_from_string( '2012-07-03','iso' ); + my $test_dt = DateTime->new( # Monday + year => 2012, + month => 7, + day => 23, + hour => 11, + minute => 53, + ); + + my $later_dt = DateTime->new( # Monday + year => 2012, + month => 9, + day => 17, + hour => 17, + minute => 30, + time_zone => 'Europe/London', + ); + + +{ ## 'Datedue' tests + + $module_context->unmock('preference'); + $module_context->mock( + 'preference', + sub { + return 'Datedue'; + } + ); + # rewind dbh session + $holidays_session->reset; + + + $cal = Koha::Calendar->new( branchcode => 'MPL' ); is($cal->addDate( $dt, $one_day_dur, 'days' ), dt_from_string('2012-07-05','iso'), @@ -146,13 +226,38 @@ my $seven_day_dur = DateTime::Duration->new( days => 7 ); '2012-07-30T11:53:00', 'Add 7 days (Datedue)' ); + is( $cal->addDate( $saturday, $one_day_dur, 'days' )->day_of_week, 1, + 'addDate skips closed Sunday (Datedue)' ); + + is( $cal->addDate($day_after_christmas, -1, 'days')->ymd(), '2012-12-24', + 'Negative call to addDate (Datedue)' ); + + ## Note that the days_between API says closed days are not considered. + ## This tests are here as an API test. + cmp_ok( $cal->days_between( $test_dt, $later_dt )->in_units('days'), + '==', 40, 'days_between calculates correctly (Days)' ); + cmp_ok( $cal->days_between( $later_dt, $test_dt )->in_units('days'), + '==', 40, 'Test parameter order not relevant (Days)' ); - ## 'Calendar' tests' - $cal = Koha::Calendar->new( TEST_MODE => 1, - days_mode => 'Calendar' ); - $cal->add_holiday( dt_from_string('2012-07-04','iso') ); +} + + +{ ## 'Calendar' tests' + + $module_context->unmock('preference'); + $module_context->mock( + 'preference', + sub { + return 'Calendar'; + } + ); + # rewind dbh session + $holidays_session->reset; + + $cal = Koha::Calendar->new( branchcode => 'MPL' ); + $dt = dt_from_string('2012-07-03','iso'); is($cal->addDate( $dt, $one_day_dur, 'days' ), @@ -160,16 +265,36 @@ my $seven_day_dur = DateTime::Duration->new( days => 7 ); 'Single day add (Calendar)' ); cmp_ok($cal->addDate( $test_dt, $seven_day_dur, 'days' ), 'eq', - '2012-07-31T11:53:00', + '2012-08-01T11:53:00', 'Add 7 days (Calendar)' ); + is( $cal->addDate( $saturday, $one_day_dur, 'days' )->day_of_week, 1, + 'addDate skips closed Sunday (Calendar)' ); + + is( $cal->addDate($day_after_christmas, -1, 'days')->ymd(), '2012-12-24', + 'Negative call to addDate (Calendar)' ); + cmp_ok( $cal->days_between( $test_dt, $later_dt )->in_units('days'), + '==', 40, 'days_between calculates correctly (Calendar)' ); + + cmp_ok( $cal->days_between( $later_dt, $test_dt )->in_units('days'), + '==', 40, 'Test parameter order not relevant (Calendar)' ); +} - ## 'Days' tests - $cal = Koha::Calendar->new( TEST_MODE => 1, - days_mode => 'Days' ); - $cal->add_holiday( dt_from_string('2012-07-04','iso') ); +{ ## 'Days' tests + $module_context->unmock('preference'); + $module_context->mock( + 'preference', + sub { + return 'Days'; + } + ); + # rewind dbh session + $holidays_session->reset; + + $cal = Koha::Calendar->new( branchcode => 'MPL' ); + $dt = dt_from_string('2012-07-03','iso'); is($cal->addDate( $dt, $one_day_dur, 'days' ), @@ -179,3 +304,19 @@ my $seven_day_dur = DateTime::Duration->new( days => 7 ); cmp_ok($cal->addDate( $test_dt, $seven_day_dur, 'days' ),'eq', '2012-07-30T11:53:00', 'Add 7 days (Days)' ); + + is( $cal->addDate( $saturday, $one_day_dur, 'days' )->day_of_week, 7, + 'addDate doesn\'t skip closed Sunday (Days)' ); + + is( $cal->addDate($day_after_christmas, -1, 'days')->ymd(), '2012-12-25', + 'Negative call to addDate (Days)' ); + + ## Note that the days_between API says closed days are not considered. + ## This tests are here as an API test. + cmp_ok( $cal->days_between( $test_dt, $later_dt )->in_units('days'), + '==', 40, 'days_between calculates correctly (Days)' ); + + cmp_ok( $cal->days_between( $later_dt, $test_dt )->in_units('days'), + '==', 40, 'Test parameter order not relevant (Days)' ); + +} -- 2.39.5