Bug 14362

Summary: PEGI 15 Circulation/AgeRestrictionMarkers test fails
Product: Koha Reporter: Mark Tompsett <mtompset>
Component: Test SuiteAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Galen Charlton <gmcharlt>
Severity: normal    
Priority: P5 - low CC: chris, indradg, jonathan.druart, julian.maurice, kyle, tomascohen
Version: MainKeywords: dependency
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11999
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
My test script
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails
Bug 14362: Regression tests
Bug 14632: Add Copyright for the Koha Dev Team

Description Mark Tompsett 2015-06-08 01:23:35 UTC
In C4/Circulation.pm there is a function call to Date_to_Days(Today). This includes/excludes (not sure which) the timezone, which causes a calculation error when trying to run prove t/Circulation/AgeRestrictionMarkers.t
Comment 1 Mark Tompsett 2015-06-08 01:36:00 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2015-06-08 04:20:30 UTC Comment hidden (obsolete)
Comment 3 Mark Tompsett 2015-06-08 04:26:05 UTC Comment hidden (obsolete)
Comment 4 Mark Tompsett 2015-06-08 04:26:09 UTC Comment hidden (obsolete)
Comment 5 Mark Tompsett 2015-06-08 04:43:44 UTC Comment hidden (obsolete)
Comment 6 Mark Tompsett 2015-06-13 23:55:44 UTC
I haven't made sense of the number of failures related to timezone, but it does fail at least once, so that is all that matters in order to prevent regression.
Comment 7 Indranil Das Gupta 2015-06-14 01:08:06 UTC Comment hidden (obsolete)
Comment 8 Indranil Das Gupta 2015-06-14 01:10:40 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2015-06-18 15:47:07 UTC
Could you please detail the issue, I don't get it.

You should also add the new deps to C4/Installer/PerlDependencies.pm

Isn't it possible to use DateTime?
Comment 10 Mark Tompsett 2015-06-18 17:32:37 UTC Comment hidden (obsolete)
Comment 11 Mark Tompsett 2015-06-18 17:32:46 UTC Comment hidden (obsolete)
Comment 12 Tomás Cohen Arazi 2015-09-01 12:48:27 UTC
Mark, the fix looks good, but Test::More provides a convenient syntax for
- skipping tests
- providing a convenient warning (thus no need for the diag)

Take a look here:

http://search.cpan.org/~exodist/Test-Simple-1.001014/lib/Test/More.pm#Conditional_tests

Look at t/Calendar.t for an example.
Comment 13 Mark Tompsett 2015-09-01 14:27:12 UTC Comment hidden (obsolete)
Comment 14 Mark Tompsett 2015-09-01 14:27:18 UTC Comment hidden (obsolete)
Comment 15 Mark Tompsett 2015-09-01 15:09:10 UTC Comment hidden (obsolete)
Comment 16 Mark Tompsett 2015-09-01 15:09:22 UTC Comment hidden (obsolete)
Comment 17 Mark Tompsett 2015-09-01 15:12:51 UTC
Created attachment 42174 [details]
My test script

Just to test all 24 hours.
Makes more comprehensive testing easier. :)
Comment 18 Nick Clemens 2016-04-22 00:34:14 UTC Comment hidden (obsolete)
Comment 19 Nick Clemens 2016-04-22 00:34:19 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2016-04-23 06:33:09 UTC
Created attachment 50593 [details] [review]
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails

It is best to test when UTC date is a date in the future compared
to your timezone. I'm in Eastern, so right now, I expect this
test to fail for another 2.5 hours.

TEST PLAN
---------
1) prove t/Circulation/AgeRestrictionMarkers.t
   -- fails for PEGI 15 after 9pm.
2) Apply patch
3) prove t/Circulation/AgeRestrictionMarkers.t
   -- works.
4) koha qa test tools

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 21 Jonathan Druart 2016-04-23 06:33:15 UTC
Created attachment 50594 [details] [review]
Bug 14362: Regression tests

This should trigger the error. Attempts to shift system time
zones did not make sense as to the number of failures.

Added Time::Fake dependency, if it isn't installed these extra
tests don't run. There is a nice skip message about it.

Added License text.

TEST PLAN
---------
 1) apply test patch
 2) sudo dpkg-reconfigure tzdata
    -- set your system time to GMT (Africa/Abidjan)
 3) prove t/Circulation/AgeRestrictionMarkers.t
    -- should not fail, even if you change system
       time to any time.
 4) sudo dpkg-reconfigure tzdata
    -- set your timezone to Eastern
 5) sudo date -s"2015-06-18 21:15:00"
 6) date
    -- should be past 9pm Eastern timezone
 7) prove t/Circulation/AgeRestrictionMarkers.t
    -- kaboom!
 8) sudo date -s"2015-06-18 12:00:00"
 9) date
    -- should be noon Eastern timezone
10) prove t/Circulation/AgeRestrictionMarkers.t
    -- success?! Time sensitive tests are bad tests.
11) sudo apt-get install libtime-fake-perl
12) prove t/Circulation/AgeRestrictionMarkers.t
    -- kaboom!
    -- changing timezone to anything other than GMT
       should trigger a kaboom.
13) apply fix patch
14) prove t/Circulation/AgeRestrictionMarkers.t
    -- should work all the time.
15) less t/Circulation/AgeRestrictionMarkers.t
    -- the license text should be similar to
       http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence
16) koha qa test tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 22 Jonathan Druart 2016-04-23 06:33:18 UTC
Created attachment 50595 [details] [review]
Bug 14632: Add Copyright for the Koha Dev Team

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 23 Kyle M Hall 2016-04-29 16:53:47 UTC
prove t/Circulation/AgeRestrictionMarkers.t passes all tests for me. Is this patch set still valid?
Comment 24 Mark Tompsett 2016-04-29 16:58:16 UTC
(In reply to Kyle M Hall from comment #23)
> prove t/Circulation/AgeRestrictionMarkers.t passes all tests for me. Is this
> patch set still valid?

Yes it is. Wait until UTC date != Your date.
Run through my test script which I attached. You should be able to trigger the error that way.
Comment 25 Kyle M Hall 2016-04-29 18:00:36 UTC
(In reply to M. Tompsett from comment #24)
> (In reply to Kyle M Hall from comment #23)
> > prove t/Circulation/AgeRestrictionMarkers.t passes all tests for me. Is this
> > patch set still valid?
> 
> Yes it is. Wait until UTC date != Your date.
> Run through my test script which I attached. You should be able to trigger
> the error that way.

Thanks, I was looking at the wrong test plan!
Comment 26 Kyle M Hall 2016-04-29 18:07:54 UTC
Pushed to master for Koha 16.05, thanks Mark!
Comment 27 Julian Maurice 2016-05-09 10:00:17 UTC
Patches pushed to 3.22.x, will be in 3.22.7
Comment 28 Chris Cormack 2016-06-19 22:08:08 UTC
Pushed to 3.20.x will be in 3.20.11