Moodle
  1. Moodle
  2. MDL-39552

Make the behat section of setup.php re-entrant safe

    Details

    • Testing Instructions:
      Hide

      Run, for example the @mod_lesson behat tests, and look to the php builtin server output.

      • Without the patch you will see the errors reported in the description (because of yui combo re-including part of setup.php).
      • With the patch applied, those errors aren't there anymore (and lesson tests continue passing).

      That's all.

      Show
      Run, for example the @mod_lesson behat tests, and look to the php builtin server output. Without the patch you will see the errors reported in the description (because of yui combo re-including part of setup.php). With the patch applied, those errors aren't there anymore (and lesson tests continue passing). That's all.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      50234

      Description

      This is a followup of MDL-39478... copying from there:

      Aha, just found a problem with your code in setup.php not being re-entrant (and "theme/yui_combo.php" requires all code in setup.php to be re-entrant).

      Error (in the :8000 web browser):

      [Tue May  7 18:19:23 2013] PHP Notice:  Constant BEHAT_SITE_RUNNING already defined in lib/setup.php on line 131
      [Tue May  7 18:19:23 2013] PHP Stack trace:
      [Tue May  7 18:19:23 2013] PHP   1. {main}() theme/yui_combo.php:0
      [Tue May  7 18:19:23 2013] PHP   2. require() theme/yui_combo.php:94
      [Tue May  7 18:19:23 2013] PHP   3. define() lib/setup.php:131
      

      So yui_combo.php includes setup.php again that will be run until the ABORT_AFTER_CONFIG_CANCEL point. And your behat setup code is before that ABORT_AFTER_CONFIG_CANCEL point.

      So, there are 2 solutions...

      • Or you move the behat code after the ABORT_AFTER_CONFIG_CANCEL point.
      • Or you make the behat code re-entrant, so the define is executed conditionally.

      I'm going to try the 2nd alternative...

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sending to peer-review. I think it's safe enough...

          Show
          Eloy Lafuente (stronk7) added a comment - Sending to peer-review. I think it's safe enough...
          Hide
          David Monllaó added a comment - - edited

          Thanks Eloy, now I see the notices (without the patch, not anymore with the patch) pity that we can not detect that with MDL-38041. Nothing dangerous in theme/yui_combo.php between require(config.php) and require(setup.php)

          Show
          David Monllaó added a comment - - edited Thanks Eloy, now I see the notices (without the patch, not anymore with the patch) pity that we can not detect that with MDL-38041 . Nothing dangerous in theme/yui_combo.php between require(config.php) and require(setup.php)
          Hide
          Dan Poltawski added a comment -

          Integrated and tested during integration.

          Before the patch, lots of:

          [Wed May  8 13:03:31 2013] PHP Notice:  Constant BEHAT_SITE_RUNNING already defined in /Users/danp/git/integration/lib/setup.php on line 131
          

          After none.

          One thing that I did notice is that at the end of the tests, we got this warning:

          [Wed May  8 13:12:45 2013] ::1:60409 Invalid request (Unexpected EOF)
          [Wed May  8 13:12:45 2013] ::1:60908 [200]: /mod/lesson/view.php?id=1&pageid=1
          [Wed May  8 13:12:50 2013] ::1:60910 Invalid request (Unexpected EOF)
          [Wed May  8 13:13:48 2013] ::1:53134 [303]: /mod/lesson/continue.php
          [Wed May  8 13:13:49 2013] ::1:53136 [200]: /mod/lesson/view.php?id=1&pageid=-9&outoftime=normal
          [Wed May  8 13:13:53 2013] ::1:53139 Invalid request (Unexpected EOF)
          [Wed May  8 13:13:53 2013] ::1:53138 Invalid request (Unexpected EOF)
          

          I assume its related to selenium closing the browser and the connection being interupted rather than any other problem.

          Show
          Dan Poltawski added a comment - Integrated and tested during integration. Before the patch, lots of: [Wed May 8 13:03:31 2013] PHP Notice: Constant BEHAT_SITE_RUNNING already defined in /Users/danp/git/integration/lib/setup.php on line 131 After none. One thing that I did notice is that at the end of the tests, we got this warning: [Wed May 8 13:12:45 2013] ::1:60409 Invalid request (Unexpected EOF) [Wed May 8 13:12:45 2013] ::1:60908 [200]: /mod/lesson/view.php?id=1&pageid=1 [Wed May 8 13:12:50 2013] ::1:60910 Invalid request (Unexpected EOF) [Wed May 8 13:13:48 2013] ::1:53134 [303]: /mod/lesson/ continue .php [Wed May 8 13:13:49 2013] ::1:53136 [200]: /mod/lesson/view.php?id=1&pageid=-9&outoftime=normal [Wed May 8 13:13:53 2013] ::1:53139 Invalid request (Unexpected EOF) [Wed May 8 13:13:53 2013] ::1:53138 Invalid request (Unexpected EOF) I assume its related to selenium closing the browser and the connection being interupted rather than any other problem.
          Hide
          Dan Poltawski added a comment -

          Please note: I didn't run the whole suite, just the suggested mod_lesson part.

          Show
          Dan Poltawski added a comment - Please note: I didn't run the whole suite, just the suggested mod_lesson part.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Did you think this day was not going to arrive ever?

          Your patience has been rewarded, yay, sent upstream, thanks!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: