Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-36315

Minimize the time-window where backup_cron_helper_testcase is failing

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.2, 2.4
    • Fix Version/s: 2.3.3
    • Component/s: Backup
    • Labels:
      None
    • Testing Instructions:
      Hide

      Set the server clock to various dates/times and execute:

      phpunit backup_cron_helper_testcase backup/util/helper/tests/cronhelper_test.php

      • November 1st, 00:30 => test execution passes (was failing before the patch).
      • November 2nd, 00:30 => test execution passes (was failing before the patch).
      • November 3rd, 00:30 => test execution passes (was failing before the patch).
      • November 4th, 00:30 => test execution FAILS (coz it's "around" when New_York DST change happens).
      • November 5th, 00:30 => test execution passes.

      You also can try with dates around Brussels DST changes (last Sunday of March and October). The test execution will only fail for some hours around 02:00 AM. Previously it was failing for up-to a week.

      Note that the exact dates/times used above have been set in one computer under Europe/Madrid TZ, so results may change if another TZ is used in the testing machine. In any case, the tests should NOT be failing for more than 48h "around" the DST change moment anymore.

      Ciao

      Show
      Set the server clock to various dates/times and execute: phpunit backup_cron_helper_testcase backup/util/helper/tests/cronhelper_test.php November 1st, 00:30 => test execution passes (was failing before the patch). November 2nd, 00:30 => test execution passes (was failing before the patch). November 3rd, 00:30 => test execution passes (was failing before the patch). November 4th, 00:30 => test execution FAILS (coz it's "around" when New_York DST change happens). November 5th, 00:30 => test execution passes. You also can try with dates around Brussels DST changes (last Sunday of March and October). The test execution will only fail for some hours around 02:00 AM. Previously it was failing for up-to a week. Note that the exact dates/times used above have been set in one computer under Europe/Madrid TZ, so results may change if another TZ is used in the testing machine. In any case, the tests should NOT be failing for more than 48h "around" the DST change moment anymore. Ciao
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This is a followup of MDL-28531.

      it has been detected that the tests added to verify the behavior of calculate_next_automated_backup() are failing "too much".

      For example, since some days ago, with the next DST change for America/New_York coming next Sunday, a bunch of tests are failing. Same applies to the Europe/Brussels one 1 week ago.

      The main reason for those longs periods getting failed tests is that the calculation of the $dst variable is performed for time() and that causes next executions scheduled to return results with DST changes applied for long periods (up to 'next Monday 18:00:00' in some cases.

      So one immediate solution to REDUCE (not to fix them forever) is to, simply, change the $dst calculation, from:

      $dst = date('I');

      to:

      $dst = date('I', $now);

      That way, the failing window is reduced a lot and only when executed "around" the DST changes (some hours before to some hours after) the unit tests will fail.

      The proposed patch applies that change in the 4 places susceptible to get a "reduced" failing window, with explanations about when the tests are supposed to continue failing.

      Ciao

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            sending this to Fred for P2P review, sure he's the man for this.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - sending this to Fred for P2P review, sure he's the man for this.
            Hide
            fred Frédéric Massart added a comment -

            Thanks for fixing my mistakes Eloy! I should have thought of the DST a little bit more.

            As you said, this will not resolve the entire DST/TZ issue, but it will greatly help for now. I'm pushing this for integration!

            Cheers!

            Show
            fred Frédéric Massart added a comment - Thanks for fixing my mistakes Eloy! I should have thought of the DST a little bit more. As you said, this will not resolve the entire DST/TZ issue, but it will greatly help for now. I'm pushing this for integration! Cheers!
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks Eloy and Fred, this has been integrated now.

            Show
            poltawski Dan Poltawski added a comment - Thanks Eloy and Fred, this has been integrated now.
            Hide
            dmonllao David Monllaó added a comment -

            It passes. Tested in master with UTC +8, adding to the specified scenarios I've tested with:

            • "4 NOV 2012 17:30:00" -> passes
            • "4 NOV 2012 10:30:00" -> fails
            Show
            dmonllao David Monllaó added a comment - It passes. Tested in master with UTC +8, adding to the specified scenarios I've tested with: "4 NOV 2012 17:30:00" -> passes "4 NOV 2012 10:30:00" -> fails
            Hide
            poltawski Dan Poltawski added a comment -

            Hurray!

            You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

            Show
            poltawski Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Nov/12