Moodle
  1. Moodle
  2. MDL-39686

Allow restarting the browser session after a timeout expires

    Details

    • Testing Instructions:
      Hide

      This issue is a bit complicated to test. The proper way to test it is using saucelabs, forcing max-duration value (the setting that manages how much can a test run) to 600 seconds for example and set $CFG->behat_restart_browser_after = 200; I've tested it in nightly01 avoiding the saucelabs 10800 seconds limit and restarting after 7200 seconds, also I've tested it locally to ensure there are no regressions and the browser is restarted as expected.

      Test 1 (no regressions)

      1. With the setting disabled (default value)
      2. Run behat with --tags @mod_forum (for example)
      3. It SHOULD finish without any exception that stops the tests execution

      Test 2 (is restarted properly)

      1. In config.php set $CFG->behat_restart_browser_after to 30, so the browser will be restarted after each test (not 100% sure but probably all the tests lasts for more than 30 seconds)
      2. Edit lib/tests/behat/behat_hooks.php, and add a die('MDL-39686'); just before $this->getSession()->restart();
      3. Run behat with --tags @mod_forum (for example, will work with anything that has more than one scenario)
      4. After finishing the first scenario it SHOULD die with a MDL-39686 message
      5. git checkout lib/tests/behat/behat_hooks.php
      6. Run behat with --tags @mod_forum (for example, will work with anything that has more than one scenario)
      7. The test run SHOULD finish as expected without any exception that stops the tests execution
      Show
      This issue is a bit complicated to test. The proper way to test it is using saucelabs, forcing max-duration value (the setting that manages how much can a test run) to 600 seconds for example and set $CFG->behat_restart_browser_after = 200; I've tested it in nightly01 avoiding the saucelabs 10800 seconds limit and restarting after 7200 seconds, also I've tested it locally to ensure there are no regressions and the browser is restarted as expected. Test 1 (no regressions) With the setting disabled (default value) Run behat with --tags @mod_forum (for example) It SHOULD finish without any exception that stops the tests execution Test 2 (is restarted properly) In config.php set $CFG->behat_restart_browser_after to 30, so the browser will be restarted after each test (not 100% sure but probably all the tests lasts for more than 30 seconds) Edit lib/tests/behat/behat_hooks.php, and add a die(' MDL-39686 '); just before $this->getSession()->restart(); Run behat with --tags @mod_forum (for example, will work with anything that has more than one scenario) After finishing the first scenario it SHOULD die with a MDL-39686 message git checkout lib/tests/behat/behat_hooks.php Run behat with --tags @mod_forum (for example, will work with anything that has more than one scenario) The test run SHOULD finish as expected without any exception that stops the tests execution
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.5 Branch:
    • Pull Master Branch:
      MDL-39686_master
    • Rank:
      50412

      Description

      Saucelabs allows 10000 seconds tests, which is not enough to run our whole suite, neither to run @_cross_browser features. We can restart the browser session after X seconds to avoid it.

        Activity

        Hide
        David Monllaó added a comment -

        Requesting peer review, not sure about adding something to core that specific to an external service, but I can't think of another workaround for the problem, another option is to apply the patch only in our CI server, but I guess we will not be the only organization using saucelabs to run their own suite

        Show
        David Monllaó added a comment - Requesting peer review, not sure about adding something to core that specific to an external service, but I can't think of another workaround for the problem, another option is to apply the patch only in our CI server, but I guess we will not be the only organization using saucelabs to run their own suite
        Hide
        David Monllaó added a comment -

        I forgot to mention, I've extended the $CFG->behat_config example in config-dist.php taking advantage of this issue, let me know if you don't like and I create a separate issue, I think is necessary because people asks for it

        Show
        David Monllaó added a comment - I forgot to mention, I've extended the $CFG->behat_config example in config-dist.php taking advantage of this issue, let me know if you don't like and I create a separate issue, I think is necessary because people asks for it
        Hide
        David Monllaó added a comment -

        Submitting for integration review as it could be good to integrate this and run the CI jobs with it

        Show
        David Monllaó added a comment - Submitting for integration review as it could be good to integrate this and run the CI jobs with it
        Hide
        Damyon Wiese 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
        Damyon Wiese 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
        Damyon Wiese added a comment -

        Hi David,

        It's a small thing but I would like a more descriptive name for the variable: laststart (IMO lastbrowsersessionstart sounds ok). Currently it looks like it is the start of behat test run.

        Since you are changing it anyway, I would prefer the examples in config-dist be split to a new issue. It is not a big deal but it is always to keep issues separate when looking back at the history.

        Thanks!

        Show
        Damyon Wiese added a comment - Hi David, It's a small thing but I would like a more descriptive name for the variable: laststart (IMO lastbrowsersessionstart sounds ok). Currently it looks like it is the start of behat test run. Since you are changing it anyway, I would prefer the examples in config-dist be split to a new issue. It is not a big deal but it is always to keep issues separate when looking back at the history. Thanks!
        Hide
        David Monllaó added a comment -

        Thanks Damyon, var name changed, rebased and MDL-39853 created to add more info to the $CFG->behat_config setting

        Show
        David Monllaó added a comment - Thanks Damyon, var name changed, rebased and MDL-39853 created to add more info to the $CFG->behat_config setting
        Hide
        Damyon Wiese added a comment -

        Thanks David,

        Looks good now. Integrated to 25 and master.

        Show
        Damyon Wiese added a comment - Thanks David, Looks good now. Integrated to 25 and master.
        Hide
        Michael de Raadt added a comment -

        I'm noting that Marina has started this test and will return to it shortly.

        Show
        Michael de Raadt added a comment - I'm noting that Marina has started this test and will return to it shortly.
        Hide
        Marina Glancy added a comment -

        works as expected, no regressions. test passed

        Show
        Marina Glancy added a comment - works as expected, no regressions. test passed
        Hide
        Damyon Wiese added a comment -

        Thanks for your hard work. This issue has now been sent upstream and will soon be downloaded by millions of Moodlers!

        Regards, Damyon

        Show
        Damyon Wiese added a comment - Thanks for your hard work. This issue has now been sent upstream and will soon be downloaded by millions of Moodlers! Regards, Damyon

          People

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

            Dates

            • Created:
              Updated:
              Resolved: