Bug 9074 - DB dependent tests for is_holiday
Summary: DB dependent tests for is_holiday
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Holidays (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 8800
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-14 14:47 UTC by Tomás Cohen Arazi
Modified: 2014-12-07 20:02 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9074 - DB dependent tests for is_holiday (14.50 KB, patch)
2012-11-14 17:29 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9074 - DB dependent tests for is_holiday (14.53 KB, patch)
2012-11-21 14:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9074 - DB dependent tests for is_holiday (14.58 KB, patch)
2012-11-21 14:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9074 - DB dependent tests for is_holiday (14.54 KB, patch)
2012-11-21 17:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9074 - DB dependent tests for is_holiday (9.90 KB, patch)
2012-11-22 12:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2012-11-14 14:47:12 UTC
We need several de-dependent tests for is_holiday. We currently only have sample data and tests for repeteable holidays. Missing:

- special holidays
- exception holidays
Comment 1 Tomás Cohen Arazi 2012-11-14 17:29:13 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2012-11-14 17:30:29 UTC
The test for holiday exceptions fails, because of bug 9078 for which I'm providing a patch in a moment.
Comment 3 Tomás Cohen Arazi 2012-11-14 17:32:09 UTC
(In reply to comment #2)
> The test for holiday exceptions fails, because of bug 9078 for which I'm
> providing a patch in a moment.

Forgot to tell that this was built against master with 8800 patch. Otherwise the test for that truncation problem fails.
Comment 4 Kyle M Hall 2012-11-21 14:03:33 UTC
git am -s /tmp/Bug-9074---DB-dependent-tests-for-isholiday-eiKuaq.patch
Applying: Bug 9074 - DB dependent tests for is_holiday
/home/koha/kohaclone/.git/rebase-apply/patch:101: trailing whitespace.
INSERT INTO `repeatable_holidays` VALUES 
/home/koha/kohaclone/.git/rebase-apply/patch:134: trailing whitespace.
SET FOREIGN_KEY_CHECKS=1;INSERT INTO `repeatable_holidays` VALUES 
/home/koha/kohaclone/.git/rebase-apply/patch:225: trailing whitespace.
INSERT INTO `repeatable_holidays` VALUES 
/home/koha/kohaclone/.git/rebase-apply/patch:282: trailing whitespace.
# is_holiday truncates the date to day as that's 
error: patch failed: installer/data/mysql/en/optional/sample_holidays.txt:1
error: installer/data/mysql/en/optional/sample_holidays.txt: patch does not apply
/home/koha/kohaclone/.git/rebase-apply/patch:258: new blank line at EOF.
+
Patch failed at 0001 Bug 9074 - DB dependent tests for is_holiday
Comment 5 Kyle M Hall 2012-11-21 14:35:53 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2012-11-21 14:39:31 UTC
Here is my output. I'm not sure if this is an error with the test, or an error in the code.

perl t/db_dependent/Holidays.t
1..11
ok 1 - use Koha::Calendar;
ok 2 - use C4::Calendar;
ok 3 - Koha::Calendar class returned isa Koha::Calendar
ok 4 - C4::Calendar class returned isa C4::Calendar
ok 5 - Sunday is a closed day
ok 6 - Monday is not a closed day
ok 7 - Christmas is a closed day
ok 8 - New Years day is a closed day
ok 9 - Sample single holiday is a closed day
not ok 10 - Sample sunday exception is not a closed day
#   Failed test 'Sample sunday exception is not a closed day'
#   at t/db_dependent/Holidays.t line 59.
#          got: '1'
#     expected: '0'
not ok 11 - is_holiday should truncate the date for holiday validation
#   Failed test 'is_holiday should truncate the date for holiday validation'
#   at t/db_dependent/Holidays.t line 73.
#          got: '0'
#     expected: '1'
# Looks like you failed 2 tests of 11.
Comment 7 Kyle M Hall 2012-11-21 14:43:01 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2012-11-21 14:44:06 UTC
Tests are failing due to bad code, the tests are good.
Comment 9 Kyle M Hall 2012-11-21 17:36:34 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2012-11-21 21:21:21 UTC
Hi Tomas, 
I am wondering if adding the holidays to all the translated sample data is necessary. Also a bit worried because it includes a branchcode, that I think we might need, but it is not there in most of the non-english sample holidays. 
Do we need to clean them up?
Comment 11 Tomás Cohen Arazi 2012-11-21 21:31:37 UTC
Katrin, we could just empty the MPL string, the only side effect would be that holidays will be set for all branches. Could it be done by the signer or should I provide a new patch?
Comment 12 Katrin Fischer 2012-11-21 21:42:17 UTC
I wonder if we should have that kind of data if it's not possible to recraete it from the interface. I tried adding a holiday with an empty branchcode - if you select 'copy to all branches' in the database an entry for each branchcode is created. So it seems our test data is wrong? I am not sure what to do here, I will concentrate on testing 8800 for now.
Comment 13 Tomás Cohen Arazi 2012-11-21 21:45:05 UTC
in conjunction with patches from 9078 please!
Comment 14 Tomás Cohen Arazi 2012-11-22 12:09:08 UTC
Created attachment 13634 [details] [review]
Bug 9074 - DB dependent tests for is_holiday

Some DB dependent tests for:

- Special holidays
- Exception holidays

I added a test for a bug introduced by 8966.
Languages that don't provide the MPL branch in their sample data are ommited.

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Katrin Fischer 2012-11-29 20:10:32 UTC
I would like to change this to 'in discussion' for now - please feel free to object. Tomas has thankfully brought up the topic on the mailing list: http://lists.koha-community.org/pipermail/koha-devel/2012-November/038525.html
Comment 16 Tomás Cohen Arazi 2013-03-04 14:08:22 UTC
This bug is not being discussed. As I said in Bug 9133 we need a data set for testing purposes. That's where this should fit instead of the default standard example values. That said, I mark this resolved wontfix. If anyone feels this should be different, just change the status.