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

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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:

      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

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt 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
            timhunt 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.
            Hide
            timhunt 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
            timhunt 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.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Integrated, thanks! I missed that, grrr.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Integrated, thanks! I missed that, grrr.
            Hide
            gerry Gerard Caulfield added a comment -

            Evidence of failed test

            Show
            gerry Gerard Caulfield added a comment - Evidence of failed test
            Hide
            gerry 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
            gerry 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.
            Hide
            gerry 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
            gerry 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
            timhunt 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
            timhunt 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.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Extra commit integrated by cherry-picking.

            This is ready for a new testing round, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Extra commit integrated by cherry-picking. This is ready for a new testing round, thanks!
            Hide
            gerry 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
            gerry 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
            gerry Gerard Caulfield added a comment -

            Test passed.

            Show
            gerry Gerard Caulfield added a comment - Test passed.
            Hide
            timhunt 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
            timhunt 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?
            Hide
            stronk7 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
            stronk7 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

              People

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

                Dates

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