Moodle
  1. Moodle
  2. MDL-28531

Automated backups run not on the time they are scheduled

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.3, 2.1, 2.2.4, 2.3.1
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide

      Test pre-requisites

      1. Enable automated backups (Settings > Site administration > Courses > Backups > Automated backup setup)
      2. Set the cron time to 6pm (or any time later than now)
      3. Enable today's day and Sunday
      4. Set your admin's user to server timezone (Settings > My profile settings > Edit profile)
      5. Your server timezone should be Australia/Perth (This is you machine setting)
      6. Tests are to be run one after the other (without reset except if specified)

      Test 1

      1. Create two new courses, one has to be hidden
      2. Run the cron
      3. Go to the backup report (Settings > Site administration > Reports > Backups)
      4. Make sure boths courses have been skipped
      5. Make sure they're set to run today at 6pm

      Test 2

      1. Create a new course
      2. Run the cron
      3. Go to the backup report
      4. Make sure all the courses have been skipped
      5. Make sure all the courses are scheduled for today at 6pm

      Test 3

      1. Change your system time to 7pm
      2. Run the cron
      3. Go to the backup report
      4. Make sure courses have been backed up
      5. Make sure courses are scheduled for Sunday at 6pm

      Test 4

      1. Reset your system to the correct time/date
      2. Change your admin's timezone to UTC-2
      3. Run the cron
      4. In the cron output, make sure courses are scheduled for tomorrow at 4am
      5. Go to the backup report
      6. Make sure courses are skipped
      7. Make sure they are scheduled for today at 6pm

      Test 5

      1. Change your system time to tomorrow at 5am
      2. Run the cron
      3. In the cron output, make sure courses are scheduled for Monday, 4 am
      4. Go to the backup report
      5. Make sure courses are backed up
      6. Make sure they are rescheduled for Sunday at 6am

      Test 6

      1. Run the Backup Unit Tests and make sure none fails
      2. `phpunit` or Site Administration > Development > Unit Tests in 2.2
      Show
      Test pre-requisites Enable automated backups (Settings > Site administration > Courses > Backups > Automated backup setup) Set the cron time to 6pm (or any time later than now) Enable today's day and Sunday Set your admin's user to server timezone (Settings > My profile settings > Edit profile) Your server timezone should be Australia/Perth (This is you machine setting) Tests are to be run one after the other (without reset except if specified) Test 1 Create two new courses, one has to be hidden Run the cron Go to the backup report (Settings > Site administration > Reports > Backups) Make sure boths courses have been skipped Make sure they're set to run today at 6pm Test 2 Create a new course Run the cron Go to the backup report Make sure all the courses have been skipped Make sure all the courses are scheduled for today at 6pm Test 3 Change your system time to 7pm Run the cron Go to the backup report Make sure courses have been backed up Make sure courses are scheduled for Sunday at 6pm Test 4 Reset your system to the correct time/date Change your admin's timezone to UTC-2 Run the cron In the cron output, make sure courses are scheduled for tomorrow at 4am Go to the backup report Make sure courses are skipped Make sure they are scheduled for today at 6pm Test 5 Change your system time to tomorrow at 5am Run the cron In the cron output, make sure courses are scheduled for Monday, 4 am Go to the backup report Make sure courses are backed up Make sure they are rescheduled for Sunday at 6am Test 6 Run the Backup Unit Tests and make sure none fails `phpunit` or Site Administration > Development > Unit Tests in 2.2
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-28531-master
    • Rank:
      18189

      Description

      There are some bugs in backup_cron_automated_helper::calculate_next_automated_backup, causing the errors in calculation of the next backup time.

      Also if the automatic backup is enabled but no week days for it are selected, the backup still runs.

      Patch and unittest are provided.

      1. MDL-28531.patch
        2 kB
        Marina Glancy
      2. testnextautobackup.php
        7 kB
        Marina Glancy

        Issue Links

          Activity

          Hide
          Marina Glancy added a comment -

          Patch that fixes the bugs in calculation of the next backup time.
          It also adds a check that schedule exists before running automatic backups

          Show
          Marina Glancy added a comment - Patch that fixes the bugs in calculation of the next backup time. It also adds a check that schedule exists before running automatic backups
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sorry, I think there is some misinterpretation here. Let's see and agree:

          1) Automated backups can be executed by two different scripts:

          1. By main Moodle cron (web or cli). In this case they are executed with directive RUN_ON_SCHEDULE, and then is mandatory to have some weekday(s) defined.
          2. By own cli script (automated_backups.php). In this case automated backups are executed with directive RUN_IMMEDIATELY, and then is not needed to have any weekday defined. The automated backup simply backups all the courses and done.

          So, your new $weekdays check seems incorrect, because it's setting ALL runs to disabled and only scheduled ones should.

          2) Based on the previous point, it's perfectly possible to have one site without weekdays defined (aka "00000000"). In those cases, with your changes, calculate_next_automated_backup() won't perform ever the "recursive" call, because $result will be 0 always. So it may end with some date in the past (and that seems incorrect). In the other side, artificially going to the next day (current approach) also seems incorrect.

          So, perhaps, the really best solution is to avoid calculate_next_automated_backup() to be called when running one RUN_IMMEDIATELY automated backup. Only RUN_ON_SCHEDULE backups should be handling next execution times.

          And that is. Really this needs some clarification / reorganization to work better.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Sorry, I think there is some misinterpretation here. Let's see and agree: 1) Automated backups can be executed by two different scripts: By main Moodle cron (web or cli). In this case they are executed with directive RUN_ON_SCHEDULE, and then is mandatory to have some weekday(s) defined. By own cli script (automated_backups.php). In this case automated backups are executed with directive RUN_IMMEDIATELY, and then is not needed to have any weekday defined. The automated backup simply backups all the courses and done. So, your new $weekdays check seems incorrect, because it's setting ALL runs to disabled and only scheduled ones should. 2) Based on the previous point, it's perfectly possible to have one site without weekdays defined (aka "00000000"). In those cases, with your changes, calculate_next_automated_backup() won't perform ever the "recursive" call, because $result will be 0 always. So it may end with some date in the past (and that seems incorrect). In the other side, artificially going to the next day (current approach) also seems incorrect. So, perhaps, the really best solution is to avoid calculate_next_automated_backup() to be called when running one RUN_IMMEDIATELY automated backup. Only RUN_ON_SCHEDULE backups should be handling next execution times. And that is. Really this needs some clarification / reorganization to work better. Ciao
          Hide
          Marina Glancy added a comment -

          Eloy, I move here my comment from MDL-25454, because we decided to change calculation of the next automatic backup time in this issue:

          During working on this issue I have noticed some differences between 1.9 and 2.0+

          In 1.9 the field backup_course.nextstarttime is updated each time the cron runs. In 2.0 it is updated only when the backup is performed. Probably because of this the check backup_course.nextstarttime>0 is replaced in 2.0 with backup_course.nextstarttime>=0.

          What it means (in 2.0+):

          • if course was hidden, the nextstarttime may never be updated. This was changed in this issue and it will most likely resolve MDL-28242
          • after the automatic backup is turned on for the first time, the backup is performed for all courses on the next cron run, which easily may be in the middle of the day slowing the work of the big website (quite unexpectedly for admin)
          • if the automatic backup schedule was changed, the change will only be applied after the next run which is already scheduled (again, admin may be unhappy)

          Probably it causes the problems for Emma who created issue MDL-25994 (if 1/10/2011 is the 1st of October in her schedule)

          The fix for it is really small. This is the patch for 2.0+:

          --- a/backup/util/helper/backup_cron_helper.class.php
          +++ b/backup/util/helper/backup_cron_helper.class.php
          @@ -124,7 +124,12 @@ abstract class backup_cron_automated_helper {
                           }
           
                           // Skip courses that do not yet need backup
          -                $skipped = !(($backupcourse->nextstarttime >= 0 && $backupcourse->nextstarttime < $now) || $rundirective == self::RUN_IMMEDIATELY);
          +                $skipped = !(($backupcourse->nextstarttime > 0 && $backupcourse->nextstarttime < $now) || $rundirective == self::RUN_IMMEDIATELY);
          +                if ($skipped && $backupcourse->nextstarttime != $nextstarttime) {
          +                    // Update nextstarttime in DB if the schedule has been changed but this time backup is skipped
          +                    $backupcourse->nextstarttime = $nextstarttime;
          +                    $DB->update_record('backup_courses', $backupcourse);
          +                }
                           // Skip backup of unavailable courses that have remained unmodified in a month
                           if (!$skipped && empty($course->visible) && ($now - $course->timemodified) > 31*24*60*60) {  //Hidden + settings were unmodified last month
                               //Check log if there were any modifications to the course content
          
          Show
          Marina Glancy added a comment - Eloy, I move here my comment from MDL-25454 , because we decided to change calculation of the next automatic backup time in this issue: During working on this issue I have noticed some differences between 1.9 and 2.0+ In 1.9 the field backup_course.nextstarttime is updated each time the cron runs. In 2.0 it is updated only when the backup is performed. Probably because of this the check backup_course.nextstarttime>0 is replaced in 2.0 with backup_course.nextstarttime>=0. What it means (in 2.0+): if course was hidden, the nextstarttime may never be updated. This was changed in this issue and it will most likely resolve MDL-28242 after the automatic backup is turned on for the first time, the backup is performed for all courses on the next cron run, which easily may be in the middle of the day slowing the work of the big website (quite unexpectedly for admin) if the automatic backup schedule was changed, the change will only be applied after the next run which is already scheduled (again, admin may be unhappy) Probably it causes the problems for Emma who created issue MDL-25994 (if 1/10/2011 is the 1st of October in her schedule) The fix for it is really small. This is the patch for 2.0+: --- a/backup/util/helper/backup_cron_helper.class.php +++ b/backup/util/helper/backup_cron_helper.class.php @@ -124,7 +124,12 @@ abstract class backup_cron_automated_helper { } // Skip courses that do not yet need backup - $skipped = !(($backupcourse->nextstarttime >= 0 && $backupcourse->nextstarttime < $now) || $rundirective == self::RUN_IMMEDIATELY); + $skipped = !(($backupcourse->nextstarttime > 0 && $backupcourse->nextstarttime < $now) || $rundirective == self::RUN_IMMEDIATELY); + if ($skipped && $backupcourse->nextstarttime != $nextstarttime) { + // Update nextstarttime in DB if the schedule has been changed but this time backup is skipped + $backupcourse->nextstarttime = $nextstarttime; + $DB->update_record('backup_courses', $backupcourse); + } // Skip backup of unavailable courses that have remained unmodified in a month if (!$skipped && empty($course->visible) && ($now - $course->timemodified) > 31*24*60*60) { //Hidden + settings were unmodified last month //Check log if there were any modifications to the course content
          Hide
          Michael de Raadt added a comment -

          I'm bumping this issue as it has been duplicated a number of times.

          There definitely seems to be a problem with the timing of automated backups.

          Show
          Michael de Raadt added a comment - I'm bumping this issue as it has been duplicated a number of times. There definitely seems to be a problem with the timing of automated backups.
          Hide
          Michael de Raadt added a comment -

          There is a lot of additional information in the linked duplicate issues.

          Show
          Michael de Raadt added a comment - There is a lot of additional information in the linked duplicate issues.
          Hide
          Adrian Greeve added a comment -

          fix estimate: 2 hours.

          Show
          Adrian Greeve added a comment - fix estimate: 2 hours.
          Hide
          Frédéric Massart added a comment -

          Assigning this issue to me

          Show
          Frédéric Massart added a comment - Assigning this issue to me
          Hide
          Frédéric Massart added a comment -

          Adding my branches here, I reckon the code is pretty much done but I'd like to add some more backup tests before pushing for peer review.

          Show
          Frédéric Massart added a comment - Adding my branches here, I reckon the code is pretty much done but I'd like to add some more backup tests before pushing for peer review.
          Hide
          Michael de Raadt added a comment -

          Carrying over to the next sprint.

          Show
          Michael de Raadt added a comment - Carrying over to the next sprint.
          Hide
          Frédéric Massart added a comment -

          Sprint planning.
          Estimated remaining development difficulty: S

          Show
          Frédéric Massart added a comment - Sprint planning. Estimated remaining development difficulty: S
          Hide
          Frédéric Massart added a comment -

          Up for peer review. Those branches probably fix most of the scheduled backup problems. The only known issue that I am aware of is the handling of Daylight Saving Time when the admin user uses another timezone than the server. This is a Moodle issue, see moodlelib.php:2250 get_user_timezone_offset().

          • Next automated backup time can be on the same day (if in future)
          • Handles future/past according to the user, not the server
          • On first cron run, a new course will never be backed up but scheduled (this is not a bug)
          Show
          Frédéric Massart added a comment - Up for peer review. Those branches probably fix most of the scheduled backup problems. The only known issue that I am aware of is the handling of Daylight Saving Time when the admin user uses another timezone than the server. This is a Moodle issue, see moodlelib.php:2250 get_user_timezone_offset(). Next automated backup time can be on the same day (if in future) Handles future/past according to the user, not the server On first cron run, a new course will never be backed up but scheduled (this is not a bug)
          Hide
          Rajesh Taneja added a comment -

          Patch looks Grt Fred,

          Although you might want to fix few minor docs.

          1. Spell mistake https://github.com/FMCorz/moodle/compare/MDL-28531-master#L0R283
          2. Will be nice to have comment at https://github.com/FMCorz/moodle/compare/MDL-28531-master#L1R2318

          Changing return value (https://github.com/FMCorz/moodle/compare/MDL-28531-master#L0R287) is fine, but we could have done without it. Anyway, don't see any repercussions as it's just used once in backup.

          FYI: dst is only valid for string timezones and they get populated manually from location -> update timezones and AFAIK they work fine.

          Show
          Rajesh Taneja added a comment - Patch looks Grt Fred, Although you might want to fix few minor docs. Spell mistake https://github.com/FMCorz/moodle/compare/MDL-28531-master#L0R283 Will be nice to have comment at https://github.com/FMCorz/moodle/compare/MDL-28531-master#L1R2318 Changing return value ( https://github.com/FMCorz/moodle/compare/MDL-28531-master#L0R287 ) is fine, but we could have done without it. Anyway, don't see any repercussions as it's just used once in backup. FYI: dst is only valid for string timezones and they get populated manually from location -> update timezones and AFAIK they work fine.
          Hide
          Frédéric Massart added a comment -

          Thanks for your thorough review Raj. I added a few comments am pushing for integration.

          Show
          Frédéric Massart added a comment - Thanks for your thorough review Raj. I added a few comments am pushing for integration.
          Hide
          Aparup Banerjee added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Aparup Banerjee added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

          This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P
          Apologies for the inconvenience, this will be integrated next week. Thanks for your collaboration & ciao

          Show
          Dan Poltawski added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologies for the inconvenience, this will be integrated next week. Thanks for your collaboration & ciao
          Hide
          Aparup Banerjee added a comment -

          Hi Fred,
          This looks fine to me to integrate .. But i did notice that a whole bunch of tests in lib/tests/backup_test.php would seem better placed in a backup/util/helper/tests/cronhelper_test.php perhaps. I think we should decide where this needs to be before integrating this.

          Show
          Aparup Banerjee added a comment - Hi Fred, This looks fine to me to integrate .. But i did notice that a whole bunch of tests in lib/tests/backup_test.php would seem better placed in a backup/util/helper/tests/cronhelper_test.php perhaps. I think we should decide where this needs to be before integrating this.
          Hide
          Frédéric Massart added a comment -

          Hi Apu, I have created MDL-34756 to move the Unit Tests so that this issue can still be integrated this week. Thanks!

          Show
          Frédéric Massart added a comment - Hi Apu, I have created MDL-34756 to move the Unit Tests so that this issue can still be integrated this week. Thanks!
          Hide
          Aparup Banerjee added a comment -

          some other unit tests failing (guess API changes do deserver more than just local phpunit test runs)

          moodlelib_testcase::test_make_timestamp
          Expected: 1309528800 => Actual: 1309514400,
          Please check if timezones are updated (Site adminstration -> location -> update timezone)
          Failed asserting that '1309514400' matches expected '1309528800'.

          /Users/Shared/Jenkins/Home/git_repositories/master/lib/tests/moodlelib_test.php:1706
          /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/advanced_testcase.php:76


          dateselector_form_element_testcase::test_exportvalue
          Please check if timezones are updated (Site adminstration -> location -> update timezone)
          Failed asserting that Array (
          'dateselector' => 1309449600
          ) is identical to Array (
          'dateselector' => 1309478400
          ).

          /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/dateselector_test.php:173
          /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64


          dateselector_form_element_testcase::test_onquickformevent
          Failed asserting that Array (
          'day' => Array (
          0 => 1
          )
          'month' => Array (
          0 => 7
          )
          'year' => Array (
          0 => 2011
          )
          ) is identical to Array (
          'day' => Array (
          0 => 30
          )
          'month' => Array (
          0 => 6
          )
          'year' => Array (
          0 => 2011
          )
          ).

          /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/dateselector_test.php:205
          /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64


          datetimeselector_form_element_testcase::test_exportvalue
          Please check if timezones are updated (Site adminstration -> location -> update timezone)
          Failed asserting that Array (
          'dateselector' => 1309449600
          ) is identical to Array (
          'dateselector' => 1309478400
          ).

          /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/datetimeselector_test.php:185
          /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64


          datetimeselector_form_element_testcase::test_onquickformevent
          Failed asserting that Array (
          'day' => Array (
          0 => 1
          )
          'month' => Array (
          0 => 7
          )
          'year' => Array (
          0 => 2011
          )
          'hour' => Array (
          0 => 0
          )
          'minute' => Array (
          0 => 0
          )
          ) is identical to Array (
          'day' => Array (
          0 => 30
          )
          'month' => Array (
          0 => 6
          )
          'year' => Array (
          0 => 2011
          )
          'hour' => Array (
          0 => 16
          )
          'minute' => Array (
          0 => 0
          )
          ).

          /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/datetimeselector_test.php:219
          /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:6

          Show
          Aparup Banerjee added a comment - some other unit tests failing (guess API changes do deserver more than just local phpunit test runs) moodlelib_testcase::test_make_timestamp Expected: 1309528800 => Actual: 1309514400, Please check if timezones are updated (Site adminstration -> location -> update timezone) Failed asserting that '1309514400' matches expected '1309528800'. /Users/Shared/Jenkins/Home/git_repositories/master/lib/tests/moodlelib_test.php:1706 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/advanced_testcase.php:76 — dateselector_form_element_testcase::test_exportvalue Please check if timezones are updated (Site adminstration -> location -> update timezone) Failed asserting that Array ( 'dateselector' => 1309449600 ) is identical to Array ( 'dateselector' => 1309478400 ). /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/dateselector_test.php:173 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64 dateselector_form_element_testcase::test_onquickformevent Failed asserting that Array ( 'day' => Array ( 0 => 1 ) 'month' => Array ( 0 => 7 ) 'year' => Array ( 0 => 2011 ) ) is identical to Array ( 'day' => Array ( 0 => 30 ) 'month' => Array ( 0 => 6 ) 'year' => Array ( 0 => 2011 ) ). /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/dateselector_test.php:205 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64 datetimeselector_form_element_testcase::test_exportvalue Please check if timezones are updated (Site adminstration -> location -> update timezone) Failed asserting that Array ( 'dateselector' => 1309449600 ) is identical to Array ( 'dateselector' => 1309478400 ). /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/datetimeselector_test.php:185 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:64 datetimeselector_form_element_testcase::test_onquickformevent Failed asserting that Array ( 'day' => Array ( 0 => 1 ) 'month' => Array ( 0 => 7 ) 'year' => Array ( 0 => 2011 ) 'hour' => Array ( 0 => 0 ) 'minute' => Array ( 0 => 0 ) ) is identical to Array ( 'day' => Array ( 0 => 30 ) 'month' => Array ( 0 => 6 ) 'year' => Array ( 0 => 2011 ) 'hour' => Array ( 0 => 16 ) 'minute' => Array ( 0 => 0 ) ). /Users/Shared/Jenkins/Home/git_repositories/master/lib/form/tests/datetimeselector_test.php:219 /Users/Shared/Jenkins/Home/git_repositories/master/lib/phpunit/classes/basic_testcase.php:6
          Hide
          Aparup Banerjee added a comment -

          Hi Fred,
          reopening this as spoken. We need to look at the timezone related test failures above.

          Show
          Aparup Banerjee added a comment - Hi Fred, reopening this as spoken. We need to look at the timezone related test failures above.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Michael de Raadt added a comment -

          Carrying over to the next sprint.

          Show
          Michael de Raadt added a comment - Carrying over to the next sprint.
          Hide
          Frédéric Massart added a comment -

          Apu, I have fixed the Unit Tests in 2.2, 2.3 and master. But this might need a bit of explanation.

          In moodlelib, get_user_timezone() returns the preferred timezone for the user in this order of preference:

          • The one passed as argument
          • The timezone forced by the config
          • The timezone of the user
          • The timezone of the server (CFG->timezone)

          If any of them WAS 99, or empty, we jumped to the next one. The problem occurs for a timezone which is 0 (in London, or UTC+0), then as it is empty it was skipped and often we end up with the server time.

          I have fixed this by checking for empty() AND not is_numeric().

          I don't know why the Unit Tests were wrong in the first place, but working with timezones and timestamps is really confusing.

          For a server in Perth/Australia, date('Y-m-h H:i:s', 0) returns the 1st of Jan 1970 at 8am. The same server in London would return the 1st of Jan 1970 at 12am.

          Anyway, if you need more information on those changes, poke me!

          Show
          Frédéric Massart added a comment - Apu, I have fixed the Unit Tests in 2.2, 2.3 and master. But this might need a bit of explanation. In moodlelib, get_user_timezone() returns the preferred timezone for the user in this order of preference: The one passed as argument The timezone forced by the config The timezone of the user The timezone of the server (CFG->timezone) If any of them WAS 99, or empty, we jumped to the next one. The problem occurs for a timezone which is 0 (in London, or UTC+0), then as it is empty it was skipped and often we end up with the server time. I have fixed this by checking for empty() AND not is_numeric(). I don't know why the Unit Tests were wrong in the first place, but working with timezones and timestamps is really confusing. For a server in Perth/Australia, date('Y-m-h H:i:s', 0) returns the 1st of Jan 1970 at 8am. The same server in London would return the 1st of Jan 1970 at 12am. Anyway, if you need more information on those changes, poke me!
          Hide
          Rajesh Taneja added a comment -

          Seems fine to me. Pushing it back for integration review.

          Show
          Rajesh Taneja added a comment - Seems fine to me. Pushing it back for integration review.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! (22, 23 & master)

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! (22, 23 & master)
          Hide
          Jason Fowler added a comment -

          looks good Fred

          Show
          Jason Fowler added a comment - looks good Fred
          Hide
          Eloy Lafuente (stronk7) added a comment -

          YEAR!*

          CAF*, TOT!*

          • Your effort amazingly resulted. (unbelievable :-P)
          • Closing as fixed.
          • Tons of thanks.
          Show
          Eloy Lafuente (stronk7) added a comment - YEAR!* CAF*, TOT!* Your effort amazingly resulted. (unbelievable :-P) Closing as fixed. Tons of thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Note MDL-36315 has been created to improve a bit phpunit results by reducing the time-window where tests are, right now, failing (incoming New_York DST change).

          Show
          Eloy Lafuente (stronk7) added a comment - Note MDL-36315 has been created to improve a bit phpunit results by reducing the time-window where tests are, right now, failing (incoming New_York DST change).

            People

            • Votes:
              12 Vote for this issue
              Watchers:
              13 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: