Bug 14362 - PEGI 15 Circulation/AgeRestrictionMarkers test fails
Summary: PEGI 15 Circulation/AgeRestrictionMarkers test fails
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Test Suite (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: M. Tompsett
QA Contact: Galen Charlton
URL:
Keywords: dependency
Depends on:
Blocks:
 
Reported: 2015-06-08 01:23 UTC by M. Tompsett
Modified: 2017-12-07 22:17 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.31 KB, patch)
2015-06-08 01:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (3.49 KB, patch)
2015-06-08 04:20 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (3.60 KB, patch)
2015-06-08 04:26 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.31 KB, patch)
2015-06-08 04:26 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (3.55 KB, patch)
2015-06-08 04:43 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.63 KB, patch)
2015-06-14 01:08 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14362: Regression tests (3.74 KB, patch)
2015-06-14 01:10 UTC, Indranil Das Gupta
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.31 KB, patch)
2015-06-18 17:32 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (5.64 KB, patch)
2015-06-18 17:32 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.31 KB, patch)
2015-09-01 14:27 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (5.68 KB, patch)
2015-09-01 14:27 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.32 KB, patch)
2015-09-01 15:09 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 14362: Regression tests (6.59 KB, patch)
2015-09-01 15:09 UTC, M. Tompsett
Details | Diff | Splinter Review
My test script (1.89 KB, text/plain)
2015-09-01 15:12 UTC, M. Tompsett
Details
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.38 KB, patch)
2016-04-22 00:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14362: Regression tests (6.64 KB, patch)
2016-04-22 00:34 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 14362: PEGI15 Circulation/AgeRestrictionMarkers test fails (1.45 KB, patch)
2016-04-23 06:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14362: Regression tests (6.72 KB, patch)
2016-04-23 06:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14632: Add Copyright for the Koha Dev Team (883 bytes, patch)
2016-04-23 06:33 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description M. 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 M. Tompsett 2015-06-08 01:36:00 UTC Comment hidden (obsolete)
Comment 2 M. Tompsett 2015-06-08 04:20:30 UTC Comment hidden (obsolete)
Comment 3 M. Tompsett 2015-06-08 04:26:05 UTC Comment hidden (obsolete)
Comment 4 M. Tompsett 2015-06-08 04:26:09 UTC Comment hidden (obsolete)
Comment 5 M. Tompsett 2015-06-08 04:43:44 UTC Comment hidden (obsolete)
Comment 6 M. 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 M. Tompsett 2015-06-18 17:32:37 UTC Comment hidden (obsolete)
Comment 11 M. 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 M. Tompsett 2015-09-01 14:27:12 UTC Comment hidden (obsolete)
Comment 14 M. Tompsett 2015-09-01 14:27:18 UTC Comment hidden (obsolete)
Comment 15 M. Tompsett 2015-09-01 15:09:10 UTC Comment hidden (obsolete)
Comment 16 M. Tompsett 2015-09-01 15:09:22 UTC Comment hidden (obsolete)
Comment 17 M. 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 M. 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