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
Created attachment 39969 [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. 2) Apply patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- works. 4) koha qa test tools
Created attachment 39977 [details] [review] Bug 14362: Regression tests This should trigger the error. TEST PLAN --------- 1) sudo apt-get install libtime-fake-perl 2) apply test patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- should fail some times. 4) apply fix patch 5) prove t/Circulation/AgeRestrictionMarkers.t -- should work all the time. 6) koha qa test tools.
Created attachment 39978 [details] [review] Bug 14362: Regression tests This should trigger the error. If you are in Eastern (UTC -5), it should trigger 4 fails, unless you time it perfectly for on the hour, in which case you may get 5 failures. TEST PLAN --------- 1) sudo apt-get install libtime-fake-perl 2) apply test patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- should fail some times. 4) apply fix patch 5) prove t/Circulation/AgeRestrictionMarkers.t -- should work all the time. 6) koha qa test tools.
Created attachment 39979 [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. 2) Apply patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- works. 4) koha qa test tools
Created attachment 39980 [details] [review] Bug 14362: Regression tests This should trigger the error. Attempting shifting system time zones did not make sense as to the number of failures. TEST PLAN --------- 1) sudo apt-get install libtime-fake-perl 2) apply test patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- should fail some times. 4) apply fix patch 5) prove t/Circulation/AgeRestrictionMarkers.t -- should work all the time. 6) koha qa test tools.
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.
Created attachment 40111 [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. 2) Apply patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- works. 4) koha qa test tools Test Remarks (without patch) PASS - in my actual timezone i.e. IST (GMT +0530). FAIL - with timezone switched to US/Eastern Test Remarks (patch applied) PASS - in my actual timezone i.e. IST (GMT +0530). PASS - with timezone switched to US/Eastern Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Created attachment 40112 [details] [review] Bug 14362: Regression tests This should trigger the error. Attempting shifting system time zones did not make sense as to the number of failures. TEST PLAN --------- 1) sudo apt-get install libtime-fake-perl 2) apply test patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- should fail some times. 4) apply fix patch 5) prove t/Circulation/AgeRestrictionMarkers.t -- should work all the time. 6) koha qa test tools. Regression test failed without fix patch. Passed with test patch in place. Tested both scenario with multiple timezones. Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
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?
Created attachment 40325 [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. 2) Apply patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- works. 4) koha qa test tools
Created attachment 40326 [details] [review] Bug 14362: Regression tests This should trigger the error. Attempting shifting 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. However, a diag message is given about the lacking dependency. TEST PLAN --------- 1) apply test patch 2) sudo dpkg-reconfigure tzdata -- set your system time to GMT 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/AgetRestrictionMarkers.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) koha qa test tools.
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.
Created attachment 42163 [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. 2) Apply patch 3) prove t/Circulation/AgeRestrictionMarkers.t -- works. 4) koha qa test tools
Created attachment 42164 [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. TEST PLAN --------- 1) apply test patch 2) sudo dpkg-reconfigure tzdata -- set your system time to GMT 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) koha qa test tools.
Created attachment 42172 [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
Created attachment 42173 [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.
Created attachment 42174 [details] My test script Just to test all 24 hours. Makes more comprehensive testing easier. :)
Created attachment 50555 [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>
Created attachment 50556 [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>
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>
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>
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>
prove t/Circulation/AgeRestrictionMarkers.t passes all tests for me. Is this patch set still valid?
(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.
(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!
Pushed to master for Koha 16.05, thanks Mark!
Patches pushed to 3.22.x, will be in 3.22.7
Pushed to 3.20.x will be in 3.20.11