Moodle
  1. Moodle
  2. MDL-31157

quiz module generate an error during cron job in Moodle 2.2.1+ (Build: 20120112)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.2.2
    • Component/s: Quiz
    • Labels:
      None
    • Database:
      Any
    • Testing Instructions:
      Hide

      Upgrade to 2.2.1+ (Build: 20120112)
      run ../cli/cron.php
      Check log files

      Show
      Upgrade to 2.2.1+ (Build: 20120112) run ../cli/cron.php Check log files
    • Workaround:
      Hide

      add $timenow = time();
      in file /mod/quiz/lib.php before line 450.

      Show
      add $timenow = time(); in file /mod/quiz/lib.php before line 450.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE
    • Pull from Repository:
    • Rank:
      37598

      Description

      PHP Notice: Undefined variable: timenow in .../mod/quiz/lib.php on line 450
      PHP Stack trace:
      PHP 1.

      {main}

      () .../admin/cli/cron.php:0
      PHP 2. cron_run() .../admin/cli/cron.php:61
      PHP 3. quiz_cron() .../lib/cronlib.php:259

        Issue Links

          Activity

          Renaat Debleu created issue -
          Hide
          Tim Hunt added a comment -

          Doh! Sorry.

          Thanks for catching this regression, which was caused by MDL-30635. Fortunately, the quiz cron does not do anything very important (unless you have third-party plugins installed) so there is no need to panic.

          Show
          Tim Hunt added a comment - Doh! Sorry. Thanks for catching this regression, which was caused by MDL-30635 . Fortunately, the quiz cron does not do anything very important (unless you have third-party plugins installed) so there is no need to panic.
          Tim Hunt made changes -
          Field Original Value New Value
          Status Open [ 1 ] Waiting for integration review [ 10010 ]
          Pull from Repository git://github.com/timhunt/moodle
          Fix Version/s 2.2.2 [ 11552 ]
          Pull 2.2 Diff URL https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-31157_22
          Pull 2.2 Branch MDL-31157_22
          Tim Hunt made changes -
          Link This issue is a regression caused by MDL-30635 [ MDL-30635 ]
          Hide
          Tim Hunt added a comment -

          To INTEGRATORS: this regression only affects the 2.2 stable branch, hence the fact there is only a fix for that branch.

          Show
          Tim Hunt added a comment - To INTEGRATORS: this regression only affects the 2.2 stable branch, hence the fact there is only a fix for that branch.
          Eloy Lafuente (stronk7) made changes -
          Currently in integration Yes [ 10041 ]
          Eloy Lafuente (stronk7) made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator stronk7
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks! I missed that, grrr.

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks! I missed that, grrr.
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Gerard Caulfield made changes -
          Tester gerry
          Gerard Caulfield made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Gerard Caulfield added a comment -

          Evidence of failed test

          Show
          Gerard Caulfield added a comment - Evidence of failed test
          Gerard Caulfield made changes -
          Hide
          Gerard Caulfield added a comment - - edited

          See attachment. Unless I'm missing something, that seems to be the error this bug was asking to be fixed. Edit: or a related error.

          Show
          Gerard Caulfield added a comment - - edited See attachment. Unless I'm missing something, that seems to be the error this bug was asking to be fixed. Edit: or a related error.
          Gerard Caulfield made changes -
          Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
          Hide
          Gerard Caulfield added a comment -

          Upon further inspection the timenow error is indeed gone but the two errors shown in the screenshot did not occur for me pre integration.

          Show
          Gerard Caulfield added a comment - Upon further inspection the timenow error is indeed gone but the two errors shown in the screenshot did not occur for me pre integration.
          Hide
          Tim Hunt added a comment -

          Oh, double-Doh! There was also a global $CFG missing.

          One new branch to integrate (2.2 only): https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-31157b_22

          Apologies for my incompetence.

          Show
          Tim Hunt added a comment - Oh, double-Doh! There was also a global $CFG missing. One new branch to integrate (2.2 only): https://github.com/timhunt/moodle/compare/MOODLE_22_STABLE...MDL-31157b_22 Apologies for my incompetence.
          Eloy Lafuente (stronk7) made changes -
          Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Extra commit integrated by cherry-picking.

          This is ready for a new testing round, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Extra commit integrated by cherry-picking. This is ready for a new testing round, thanks!
          Eloy Lafuente (stronk7) made changes -
          Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
          Gerard Caulfield made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Hide
          Gerard Caulfield added a comment -

          That's fine Tim, mistakes happen. But I don't think it should have been integrated till next week. It may be easy to cherry pick something in at the last moment but testing throughly takes time and having it sent back to me at 5:30pm Perth time when I should have already left for the day doesn't lead to a good work life balance. :-/ However if I don't test it, it looks like I didn't complete all my assigned tests for the day. Last minute sending back for testing will only encourage people to pass stuff that shouldn't be passed.

          Show
          Gerard Caulfield added a comment - That's fine Tim, mistakes happen. But I don't think it should have been integrated till next week. It may be easy to cherry pick something in at the last moment but testing throughly takes time and having it sent back to me at 5:30pm Perth time when I should have already left for the day doesn't lead to a good work life balance. :-/ However if I don't test it, it looks like I didn't complete all my assigned tests for the day. Last minute sending back for testing will only encourage people to pass stuff that shouldn't be passed.
          Hide
          Gerard Caulfield added a comment -

          Test passed.

          Show
          Gerard Caulfield added a comment - Test passed.
          Gerard Caulfield made changes -
          Status Testing in progress [ 10011 ] Tested [ 10006 ]
          Hide
          Tim Hunt added a comment -

          Well, it depends on the level of risk in the proposed fix. My fix (adding a global variable declaration) is almost certain not to break anything, and almost certain to fix the problem. Therefore, it seems obvious to me that it should be integrated this week.

          If you are that desperate to go home, can't you just push this issue back into 'waiting for testing' state?

          Show
          Tim Hunt added a comment - Well, it depends on the level of risk in the proposed fix. My fix (adding a global variable declaration) is almost certain not to break anything, and almost certain to fix the problem. Therefore, it seems obvious to me that it should be integrated this week. If you are that desperate to go home, can't you just push this issue back into 'waiting for testing' state?
          Gerard Caulfield made changes -
          Comment [ If it didn't get tested I'd be responsible. Also there is a big ephasis on numbers here. :/ ]
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now available in the git and cvs repositories.

          Consider the responsibility of your fingerprints engraved there for future generations!

          Thanks for the work, closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This is now available in the git and cvs repositories. Consider the responsibility of your fingerprints engraved there for future generations! Thanks for the work, closing, ciao
          Eloy Lafuente (stronk7) made changes -
          Status Tested [ 10006 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Currently in integration Yes [ 10041 ]
          Integration date 19/Jan/12

            People

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

              Dates

              • Created:
                Updated:
                Resolved: