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

Make Behat more flexible about the wwwroot it uses to access the test site

    Details

    • Testing Instructions:
      Hide

      We should test that all that combinations are working

      Note that as "production site" I mean the normal environment, and as "test site" the one titled as "Acceptance test site".

      Test 0, install behat

      1. Install behat or util.php --drop & util.php --install & util.php --enable if it was already installed
      2. It SHOULD finish as expected

      Test 1, switch completely works as expected

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        $CFG->behat_switchcompletely = true;
        //$CFG->behat_wwwroot = http://YOURIP/path/to/your/site
      2. Go to http://localhost/path/to/your/site
      3. You SHOULD see your test site and the page SHOULD be styled as usual
      4. Follow Log in link
      5. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      6. Go to http://YOURIP/path/to/your/site
      7. You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like:
        Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry.
        Please notify server administrator.
      8. Run php admin/tool/behat/cli/util.php --enable
      9. It SHOULD finish as expected providing a command to run
      10. Copy & paste the command
      11. Wait until the first scenario finishes (or wait 1 minute instead)
      12. It SHOULD pass
      13. You can stop it know with Cntl + C

      Test 2, switch completely overwrites behat_wwwroot

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        $CFG->behat_switchcompletely = true;
        $CFG->behat_wwwroot = http://YOURIP/path/to/your/site
      2. Go to http://localhost/path/to/your/site
      3. You SHOULD see your test site and the page SHOULD be styled as usual
      4. Follow Log in link
      5. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      6. Go to http://YOURIP/path/to/your/site
      7. You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like:
        Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry.
        Please notify server administrator.
      8. Run php admin/tool/behat/cli/util.php --enable
      9. It SHOULD finish as expected providing a command to run
      10. Copy & paste the command
      11. Wait until the first scenario finishes (or wait 1 minute instead)
      12. It SHOULD pass
      13. You can stop it know with Cntl + C

      Test 3, behat_wwwroot works as expected

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        //$CFG->behat_switchcompletely = true;
        $CFG->behat_wwwroot = http://YOURIP/path/to/your/site
      2. Go to http://localhost/path/to/your/site
      3. You SHOULD see your production site
      4. Go to http://YOURIP/path/to/your/site
      5. You SHOULD see your test site and the page SHOULD be styled as usual
      6. Follow Log in link
      7. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      8. Go to http://yourmachine.host.name/path/to/your/site
      9. You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like:
        Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry.
        Please notify server administrator.
      10. Run php admin/tool/behat/cli/util.php --enable
      11. It SHOULD finish as expected providing a command to run
      12. Copy & paste the command
      13. Wait until the first scenario finishes (or wait 1 minute instead)
      14. It SHOULD pass
      15. You can stop it know with Cntl + C

      Test 4, the built-in server defaults to localhost:8000 if no behat_wwwroot is set

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        //$CFG->behat_switchcompletely = true;
        //$CFG->behat_wwwroot = http://YOURIP/path/to/your/site
      2. Start your PHP built-in server to the default URL
        cd /path/to/your/dirroot/
        php -S localhost:8000
      3. Go to http://localhost/path/to/your/site
      4. You SHOULD see your production site
      5. Go to http://localhost:8000
      6. You SHOULD see your test site and the page SHOULD be styled as usual
      7. Follow Log in link
      8. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      9. Run php admin/tool/behat/cli/util.php --enable
      10. It SHOULD finish as expected providing a command to run
      11. Copy & paste the command
      12. Wait until the first scenario finishes (or wait 1 minute instead)
      13. It SHOULD pass
      14. You can stop it know with Cntl + C

      Test 5, the built-in server can use a different URL than localhost:8000

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        //$CFG->behat_switchcompletely = true;
        $CFG->behat_wwwroot = http://localhost:8005
      2. Start your PHP built-in server to the default URL
        cd /path/to/your/dirroot/
        php -S localhost:8005
      3. Go to http://localhost/path/to/your/site
      4. You SHOULD see your production site
      5. Go to http://localhost:8005
      6. You SHOULD see your test site and the page SHOULD be styled as usual
      7. Follow Log in link
      8. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      9. Run php admin/tool/behat/cli/util.php --enable
      10. It SHOULD finish as expected providing a command to run
      11. Copy & paste the command
      12. Wait until the first scenario finishes (or wait 1 minute instead)
      13. It SHOULD pass
      14. You can stop it know with Cntl + C

      Test 6, using the built-in server, behat_wwwroot wins over it's default URL

      1. Set this in your config.php, ensure you don't have the same vars defined below/above in your config file:
        $CFG->wwwroot = 'http://localhost/path/to/your/site';
        //$CFG->behat_switchcompletely = true;
        $CFG->behat_wwwroot = http://YOURIP/path/to/your/site
      2. Start your PHP built-in server to the default URL
        cd /path/to/your/dirroot/
        php -S localhost:8000
      3. Go to http://localhost/path/to/your/site
      4. You SHOULD see your production site
      5. Go to http://localhost:8000
      6. You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like:
        Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry.
        Please notify server administrator.
      7. Go to http://YOURIP/path/to/your/site
      8. You SHOULD see your test site and the page SHOULD be styled as usual
      9. Follow Log in link
      10. You SHOULD be redirected to the login page and the page SHOULD be styled as usual
      11. Go to http://yourmachine.host.name/path/to/your/site
      12. You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like:
        Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry.
        Please notify server administrator.
      13. Run php admin/tool/behat/cli/util.php --enable
      14. It SHOULD finish as expected providing a command to run
      15. Copy & paste the command
      16. Wait until the first scenario finishes (or wait 1 minute instead)
      17. It SHOULD pass
      18. You can stop it know with Cntl + C
      Show
      We should test that all that combinations are working Note that as "production site" I mean the normal environment, and as "test site" the one titled as "Acceptance test site". Test 0, install behat Install behat or util.php --drop & util.php --install & util.php --enable if it was already installed It SHOULD finish as expected Test 1, switch completely works as expected Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; $CFG->behat_switchcompletely = true; //$CFG->behat_wwwroot = http://YOURIP/path/to/your/site Go to http://localhost/path/to/your/site You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Go to http://YOURIP/path/to/your/site You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like: Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry. Please notify server administrator. Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C Test 2, switch completely overwrites behat_wwwroot Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; $CFG->behat_switchcompletely = true; $CFG->behat_wwwroot = http://YOURIP/path/to/your/site Go to http://localhost/path/to/your/site You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Go to http://YOURIP/path/to/your/site You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like: Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry. Please notify server administrator. Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C Test 3, behat_wwwroot works as expected Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; //$CFG->behat_switchcompletely = true; $CFG->behat_wwwroot = http://YOURIP/path/to/your/site Go to http://localhost/path/to/your/site You SHOULD see your production site Go to http://YOURIP/path/to/your/site You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Go to http://yourmachine.host.name/path/to/your/site You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like: Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry. Please notify server administrator. Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C Test 4, the built-in server defaults to localhost:8000 if no behat_wwwroot is set Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; //$CFG->behat_switchcompletely = true; //$CFG->behat_wwwroot = http://YOURIP/path/to/your/site Start your PHP built-in server to the default URL cd /path/to/your/dirroot/ php -S localhost:8000 Go to http://localhost/path/to/your/site You SHOULD see your production site Go to http://localhost:8000 You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C Test 5, the built-in server can use a different URL than localhost:8000 Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; //$CFG->behat_switchcompletely = true; $CFG->behat_wwwroot = http://localhost:8005 Start your PHP built-in server to the default URL cd /path/to/your/dirroot/ php -S localhost:8005 Go to http://localhost/path/to/your/site You SHOULD see your production site Go to http://localhost:8005 You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C Test 6, using the built-in server, behat_wwwroot wins over it's default URL Set this in your config.php, ensure you don't have the same vars defined below/above in your config file: $CFG->wwwroot = 'http://localhost/path/to/your/site'; //$CFG->behat_switchcompletely = true; $CFG->behat_wwwroot = http://YOURIP/path/to/your/site Start your PHP built-in server to the default URL cd /path/to/your/dirroot/ php -S localhost:8000 Go to http://localhost/path/to/your/site You SHOULD see your production site Go to http://localhost:8000 You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like: Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry. Please notify server administrator. Go to http://YOURIP/path/to/your/site You SHOULD see your test site and the page SHOULD be styled as usual Follow Log in link You SHOULD be redirected to the login page and the page SHOULD be styled as usual Go to http://yourmachine.host.name/path/to/your/site You SHOULD be redirected to http://localhost/path/to/your/site and you SHOULD see a message like: Incorrect access detected, this server may be accessed only through "http://localhost/path/to/your/site" address, sorry. Please notify server administrator. Run php admin/tool/behat/cli/util.php --enable It SHOULD finish as expected providing a command to run Copy & paste the command Wait until the first scenario finishes (or wait 1 minute instead) It SHOULD pass You can stop it know with Cntl + C
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-41592_master

      Description

      The current methods for running Behat tests with PHP 5.3 and 5.4+ are both a bit clunky. Sure, they work for all users but don't really handle more advanced use cases.

      This patch allows you to define an alternate wwwroot for Behat. For example, creating a second Hostname/ServerAlias pointing to the same document root. No more need to boot up the in-built PHP webserver or toggle switchcompletely every time you want to run tests.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            skodak Petr Skoda added a comment -

            I like this a lot, in fact I thought it already works like this, +1 from me.

            [~davmon]: could you please peer review this?

            Show
            skodak Petr Skoda added a comment - I like this a lot, in fact I thought it already works like this, +1 from me. [~davmon] : could you please peer review this?
            Hide
            dmonllao David Monllaó added a comment -

            Hi, thanks for opening the issue and providing a patch.

            Initially we only wanted to allow the test site to run in the PHP 5.4 built-in server to prevent, as much as possible, problems with people running the tests it in a production site and not being aware of the risks of $CFG->behat_* vars, we added $CFG->behat_switchcompletely because the PHP 5.4 requirement is too restrictive for development/testing purposes; what you are proposing in here is to also allow to switch to the behat test site if (!empty($CFG->behat_wwwroot)), which is good of course, but less apocalypses-proof

            About the patch, consider looking at how setup_get_remove_url() works, it would be hard to use it as the test environment switch is done early in the startup process and lib/setuplib.php is not available. Also note that $CFG->behat_wwwroot is always set (https://github.com/moodle/moodle/blob/master/lib/setup.php#L98) so ($CFG->behat_wwwroot != 'http://localhost:8000') or something cleaner to avoid hardcoding http://localhost:8000 in more than one place would be better.

            config-dist.php and Moodle Docs (adding dev_docs_required label to the issue) should be updated according to this new behaviour.

            As it is a base change, it could also be good to add more scenarios to the testing instructions, using behat_switchcompletely, considering the PHP version...

            [Y] Syntax
            [Y] Whitespace
            [-] Output
            [-] Language
            [-] Databases
            [UPTOYOU] Testing (instructions and automated tests)
            [-] Security
            [N] Documentation
            [Y] Git
            [N] Sanity check

            Show
            dmonllao David Monllaó added a comment - Hi, thanks for opening the issue and providing a patch. Initially we only wanted to allow the test site to run in the PHP 5.4 built-in server to prevent, as much as possible, problems with people running the tests it in a production site and not being aware of the risks of $CFG->behat_* vars, we added $CFG->behat_switchcompletely because the PHP 5.4 requirement is too restrictive for development/testing purposes; what you are proposing in here is to also allow to switch to the behat test site if (!empty($CFG->behat_wwwroot)) , which is good of course, but less apocalypses-proof About the patch, consider looking at how setup_get_remove_url() works, it would be hard to use it as the test environment switch is done early in the startup process and lib/setuplib.php is not available. Also note that $CFG->behat_wwwroot is always set ( https://github.com/moodle/moodle/blob/master/lib/setup.php#L98 ) so ($CFG->behat_wwwroot != 'http://localhost:8000') or something cleaner to avoid hardcoding http://localhost:8000 in more than one place would be better. config-dist.php and Moodle Docs (adding dev_docs_required label to the issue) should be updated according to this new behaviour. As it is a base change, it could also be good to add more scenarios to the testing instructions, using behat_switchcompletely, considering the PHP version... [Y] Syntax [Y] Whitespace [-] Output [-] Language [-] Databases [UPTOYOU] Testing (instructions and automated tests) [-] Security [N] Documentation [Y] Git [N] Sanity check
            Hide
            sry_not4sale Aaron Barnes added a comment -

            FYI - this patch is quite different (an improvement I think!), so may need a bit more of a review.

            Show
            sry_not4sale Aaron Barnes added a comment - FYI - this patch is quite different (an improvement I think!), so may need a bit more of a review.
            Hide
            marina Marina Glancy added a comment -

            Hi guys, that's a really good change.

            My only concern is that we still hardcode php built-in server as 'localhost'.
            Will this still work if I run server as say "php -S marina.moodle.local:8000" ?

            And also at some moment I got an error about undefined $CFG->behat_wwwroot here:
            https://github.com/srynot4sale/moodle/blob/d2a9f16623963b198ea5afaa9c3bde48a08abfe8/lib/setup.php#L144
            but probably I was doing something wrong.

            Show
            marina Marina Glancy added a comment - Hi guys, that's a really good change. My only concern is that we still hardcode php built-in server as 'localhost'. Will this still work if I run server as say "php -S marina.moodle.local:8000" ? And also at some moment I got an error about undefined $CFG->behat_wwwroot here: https://github.com/srynot4sale/moodle/blob/d2a9f16623963b198ea5afaa9c3bde48a08abfe8/lib/setup.php#L144 but probably I was doing something wrong.
            Hide
            dmonllao David Monllaó added a comment -

            Hi Marina,

            You can already do php -S marina.moodle.local:8000 with the current codebase, what this patch does is to remove the need to use the php built-in web server if you want to allow access to both the normal site and the behat test site.

            About the undefined $CFG->behat_wwwroot I would say that you are not missing anything, if for example you are using $CFG->behat_switchcompletely $CFG->behat_wwwroot is not set as it was.

            Show
            dmonllao David Monllaó added a comment - Hi Marina, You can already do php -S marina.moodle.local:8000 with the current codebase, what this patch does is to remove the need to use the php built-in web server if you want to allow access to both the normal site and the behat test site. About the undefined $CFG->behat_wwwroot I would say that you are not missing anything, if for example you are using $CFG->behat_switchcompletely $CFG->behat_wwwroot is not set as it was.
            Hide
            dmonllao David Monllaó added a comment -

            Ups sorry, forget about the 2nd sentence of my last comment; I guess is when defined('BEHAT_TEST'), which is when the behat CLI process starts (vendor/bin/behat) and requires config.php in the before suite hook (lib/tests/behat_hooks.php)

            Show
            dmonllao David Monllaó added a comment - Ups sorry, forget about the 2nd sentence of my last comment; I guess is when defined('BEHAT_TEST') , which is when the behat CLI process starts (vendor/bin/behat) and requires config.php in the before suite hook (lib/tests/behat_hooks.php)
            Hide
            dmonllao David Monllaó added a comment -

            Adding a new review as this patch is quite different from the one I saw.

            Show
            dmonllao David Monllaó added a comment - Adding a new review as this patch is quite different from the one I saw. $CFG->behat_wwwroot needs to be set as long as $CFG->behat_switchcompletely is not, which is not happening when define('BEHAT_TEST'); We don't know whether data generators (which are used by behat) will use wwwroot or not, so we should not use the normal wwwroot, if we don't have a default and we can not guess it from _SERVER as is a CLI call, we always need a value for $CFG->behat_wwwroot As Marina commented, with this we are limiting the use of the built-in server ( https://github.com/srynot4sale/moodle/commit/d2a9f16623963b198ea5afaa9c3bde48a08abfe8#diff-7e93f51a4e3675ecc8b1b935a6e44224R113 ) it can be started using local DNSs or IPs and hardcoding localhost we avoid those usages. Here we could autofill it like in https://github.com/srynot4sale/moodle/commit/d2a9f16623963b198ea5afaa9c3bde48a08abfe8#diff-7e93f51a4e3675ecc8b1b935a6e44224R118 but as commented above, $CFG->behat_wwwroot should always be specified Could be good to add more info about the possible usages with this changes integrated (config-dist.php)
            Hide
            marina Marina Glancy added a comment -

            Aaron, I reopen it for now then. Please re-submit when David is happy with peer review (and preferably the same branch he was reviewing)

            Show
            marina Marina Glancy added a comment - Aaron, I reopen it for now then. Please re-submit when David is happy with peer review (and preferably the same branch he was reviewing)
            Hide
            cibot CiBoT added a comment -

            Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            dmonllao David Monllaó added a comment -

            Following today's chat in integration chat, I'll rework the patch and submit for integration once ready including other checkings including dataroot + realpath()

            Closing MDL-42976 as duplicate of this issue.

            Show
            dmonllao David Monllaó added a comment - Following today's chat in integration chat, I'll rework the patch and submit for integration once ready including other checkings including dataroot + realpath() Closing MDL-42976 as duplicate of this issue.
            Hide
            dmonllao David Monllaó added a comment -

            Assigning to myself and working on top of Aaron's patch keeping his contribution.

            Show
            dmonllao David Monllaó added a comment - Assigning to myself and working on top of Aaron's patch keeping his contribution.
            Hide
            sry_not4sale Aaron Barnes added a comment -

            Thanks David, I tried a couple of other approaches but couldn't settle on anything I thought that was simple, clear and performed.

            Good luck!

            Show
            sry_not4sale Aaron Barnes added a comment - Thanks David, I tried a couple of other approaches but couldn't settle on anything I thought that was simple, clear and performed. Good luck!
            Hide
            dmonllao David Monllaó added a comment -

            Attaching patch on top of Aaron's one to keep his contribution.

            I've also added:

            • Ensure CFG->behat_* is properly set when switching to behat environment; to prevent issues if the user changed the config.php vars
            • realpath() checkings in the ($CFG->behat_dataroot == $CFG->dataroot)
            Show
            dmonllao David Monllaó added a comment - Attaching patch on top of Aaron's one to keep his contribution. I've also added: Ensure CFG->behat_* is properly set when switching to behat environment; to prevent issues if the user changed the config.php vars realpath() checkings in the ($CFG->behat_dataroot == $CFG->dataroot)
            Hide
            dmonllao David Monllaó added a comment -

            Requesting review. I've split the changes in 2 because I'm not 100% sure of whether the 2nd commit should be part of this issue, another one or not done at all; in any case different than including it in this issue the testing instructions would have to be modified as Test #4 behaviour would change. Please provide feedback about it, I don't want to introduce any possible regression.

            Show
            dmonllao David Monllaó added a comment - Requesting review. I've split the changes in 2 because I'm not 100% sure of whether the 2nd commit should be part of this issue, another one or not done at all; in any case different than including it in this issue the testing instructions would have to be modified as Test #4 behaviour would change. Please provide feedback about it, I don't want to introduce any possible regression.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            It looks great... Just 2 details:

            1) It seems this is dupe: https://github.com/dmonllao/moodle/compare/moodle:master...MDL-42592_master#diff-7e93f51a4e3675ecc8b1b935a6e44224R154 (and some lines below).

            2) Could you specify here all the combinations (and settings to define) expected to be able to run behat? That would make things easier for a) documenting them in the docs clearly and b) review the patch.

            Awaiting for comments... ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - It looks great... Just 2 details: 1) It seems this is dupe: https://github.com/dmonllao/moodle/compare/moodle:master...MDL-42592_master#diff-7e93f51a4e3675ecc8b1b935a6e44224R154 (and some lines below). 2) Could you specify here all the combinations (and settings to define) expected to be able to run behat? That would make things easier for a) documenting them in the docs clearly and b) review the patch. Awaiting for comments... ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Note to me... have to look carefully to 2nd commit.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Note to me... have to look carefully to 2nd commit.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            One more thing:

            3) The commits have incorrect MDL-xxxx issue!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - One more thing: 3) The commits have incorrect MDL-xxxx issue!
            Hide
            dmonllao David Monllaó added a comment -

            Pull branches updated solving #1 and #3

            Show
            dmonllao David Monllaó added a comment - Pull branches updated solving #1 and #3
            Hide
            dmonllao David Monllaó added a comment - - edited

            All the possibilities explained, it is mostly based on the provided testing instructions:

            (In all the combinations we MUST define $CFG->behat_dataroot and $CFG->behat_prefix with different values than the production environment ones.)

            • A Use the PHP 5.4 built-in web server with it's default URL
              • The normal moodle environment can be accessed as usual through it's URL
              • The behat test site can be accessed through http://localhost:8000
              • You need to use PHP 5.4 or newer
              • How to:
                1. Start the built-in server
                  cd /to/your/moodle/dirroot
                  php -S localhost:8000
                2. php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE
            • B Use an alternative URL to the behat test site
              • The normal moodle environment can be accessed as usual through it's URL
              • The behat test site can be accessed through the value you define in $CFG->behat_wwwroot
              • How to:
                1. Settings to enable in config.php
                  $CFG->behat_wwwroot = 'http://my.local.ip.address'; // THIS IS JUST AN EXAMPLE
                2. php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE
            • C Use $CFG->wwwroot as the behat test site URL
              • The normal moodle environment can not be accessed as the URL is pointing to the behat test site
              • How to:
                1. Settings to enable in config.php
                  $CFG->behat_switchcompletely = true;
                  //$CFG->behat_wwwroot = 'http://whatever'; // THIS SETTING IS IGNORED IF beaht_switchcompletely IS SET
                2. php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE
            • D Use the PHP 5.4 built-in web server pointing to another URL you want to use
              • This option is similar to A, we just add it here to let you know that the built-in server is not restricted to work on the default localhost:8000
              • The normal moodle environment can be accessed as usual through it's URL
              • The behat test site can be accessed through the value you define in $CFG->behat_wwwroot
              • You need to use PHP 5.4 or newer
              • How to:
                1. Start the built-in server
                  cd /to/your/moodle/dirroot
                  php S {$CFG>behat_wwwroot} // WITHOUT THE http://
                2. Settings to enable in config.php
                  $CFG->behat_wwwroot = 'http://myhostname.local'; // THIS IS JUST AN EXAMPLE
                3. php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE

            As an extra, the built-in server can be used to access a moodle web server like any other web server, and even opening two php -S localhost:8000 and php -S localhost:8001 in dirroot we can use it for both production and testing environments, altough I would not recommend doing it, will be more stable to use apache or other web servers than this one.

            (Edited to rearrange options and add a comment about the compulsory settings)

            Show
            dmonllao David Monllaó added a comment - - edited All the possibilities explained, it is mostly based on the provided testing instructions: (In all the combinations we MUST define $CFG->behat_dataroot and $CFG->behat_prefix with different values than the production environment ones.) A Use the PHP 5.4 built-in web server with it's default URL The normal moodle environment can be accessed as usual through it's URL The behat test site can be accessed through http://localhost:8000 You need to use PHP 5.4 or newer How to: Start the built-in server cd /to/your/moodle/dirroot php -S localhost:8000 php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE B Use an alternative URL to the behat test site The normal moodle environment can be accessed as usual through it's URL The behat test site can be accessed through the value you define in $CFG->behat_wwwroot How to: Settings to enable in config.php $CFG->behat_wwwroot = 'http://my.local.ip.address'; // THIS IS JUST AN EXAMPLE php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE C Use $CFG->wwwroot as the behat test site URL The normal moodle environment can not be accessed as the URL is pointing to the behat test site How to: Settings to enable in config.php $CFG->behat_switchcompletely = true; //$CFG->behat_wwwroot = 'http://whatever'; // THIS SETTING IS IGNORED IF beaht_switchcompletely IS SET php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE D Use the PHP 5.4 built-in web server pointing to another URL you want to use This option is similar to A , we just add it here to let you know that the built-in server is not restricted to work on the default localhost:8000 The normal moodle environment can be accessed as usual through it's URL The behat test site can be accessed through the value you define in $CFG->behat_wwwroot You need to use PHP 5.4 or newer How to: Start the built-in server cd /to/your/moodle/dirroot php S {$CFG >behat_wwwroot} // WITHOUT THE http:// Settings to enable in config.php $CFG->behat_wwwroot = 'http://myhostname.local'; // THIS IS JUST AN EXAMPLE php admin/tool/behat/cli/util.php --enable // TO UPDATE THE BEHAT CONFIG FILE POINTING THE NEW URL TO THE SITE As an extra, the built-in server can be used to access a moodle web server like any other web server, and even opening two php -S localhost:8000 and php -S localhost:8001 in dirroot we can use it for both production and testing environments, altough I would not recommend doing it, will be more stable to use apache or other web servers than this one. (Edited to rearrange options and add a comment about the compulsory settings)
            Hide
            dmonllao David Monllaó added a comment -

            I'm changing the patch as I've discovered a PHP notice while testing. The behat utilities (install/drop/enable...) defaults to localhost:8000 in case there is no $CFG->behat_switchcompletely not $CFG->behat_wwwroot, we should keep this behaviour as we are allowing a "default behaviour".

            Show
            dmonllao David Monllaó added a comment - I'm changing the patch as I've discovered a PHP notice while testing. The behat utilities (install/drop/enable...) defaults to localhost:8000 in case there is no $CFG->behat_switchcompletely not $CFG->behat_wwwroot, we should keep this behaviour as we are allowing a "default behaviour".
            Hide
            sry_not4sale Aaron Barnes added a comment -

            To correctly test this, in each of the situations David mentions you need to:

            • Access "Acceptance testing" site in browser at the $CFG->behat_wwwroot url (and check there are no errors/notices in logs)
            • Run Behat test suit using command supplied when running util.php --enabled (check errors/notices)
            • Run util.php --drop and util.php --install (check errors/notices)
            • Access $CFG->wwwroot (except when using switch_completely) (check errors/notices)
            • Run a CLI script e.g. cron (check errors/notices)

            Show
            sry_not4sale Aaron Barnes added a comment - To correctly test this, in each of the situations David mentions you need to: Access "Acceptance testing" site in browser at the $CFG->behat_wwwroot url (and check there are no errors/notices in logs) Run Behat test suit using command supplied when running util.php --enabled (check errors/notices) Run util.php --drop and util.php --install (check errors/notices) Access $CFG->wwwroot (except when using switch_completely) (check errors/notices) Run a CLI script e.g. cron (check errors/notices)
            Hide
            sry_not4sale Aaron Barnes added a comment -

            The reason I mention this is in writing the earlier patch I came across issues in each of those situations

            Show
            sry_not4sale Aaron Barnes added a comment - The reason I mention this is in writing the earlier patch I came across issues in each of those situations
            Hide
            dmonllao David Monllaó added a comment -

            Thanks Eloy and Aaron, I've updated the patch and the testing instructions.

            I've tested with many combinations and we now the use of the PHP built-in server is not restricted to the test environment (in fact we can use 2 instances of the built-in server one as production and the other one as a behat environment)

            Show
            dmonllao David Monllaó added a comment - Thanks Eloy and Aaron, I've updated the patch and the testing instructions. I've tested with many combinations and we now the use of the PHP built-in server is not restricted to the test environment (in fact we can use 2 instances of the built-in server one as production and the other one as a behat environment)
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            This is strange, in 26_STABLE, with this applied I'm getting:

            Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot
            Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot
            Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot

            For my 3 init, --disable, --enable executions. In master I don't get them (using same config.php), that automatically builds behat_dataroot as dataroot + '_behat'.

            Have to investigate why, but have to leave now... so ttyl!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - This is strange, in 26_STABLE, with this applied I'm getting: Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot Behat config error: Define $CFG->behat_dataroot in config.php with a value different than $CFG->dataroot and $CFG->phpunit_dataroot For my 3 init, --disable, --enable executions. In master I don't get them (using same config.php), that automatically builds behat_dataroot as dataroot + '_behat'. Have to investigate why, but have to leave now... so ttyl!
            Hide
            dmonllao David Monllaó added a comment -

            Hi Eloy,

            I guess all working properly without the patch, there are two changes that may have something to do:

            • There are new realpath() checkings, so maybe there is a symbolic link or the dataroot does not exists. Could it be a test you previously did?
            • Now we check the integrity of the CFG->behat_* vars also when switching to the behat environment when running the tests
            Show
            dmonllao David Monllaó added a comment - Hi Eloy, I guess all working properly without the patch, there are two changes that may have something to do: There are new realpath() checkings, so maybe there is a symbolic link or the dataroot does not exists. Could it be a test you previously did? Now we check the integrity of the CFG->behat_* vars also when switching to the behat environment when running the tests
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Will try again. Perhaps it was an 1-time error, because also with the patch applied i ran master perfectly.

            Re-viewing this...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Will try again. Perhaps it was an 1-time error, because also with the patch applied i ran master perfectly. Re-viewing this...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            It all looks ok, the only two points I'm not 100% sure are:

            1) In admin/tool/behat/cli/util.php, it seems logically incorrect to call behat_check_config_vars() and later, set $CFG->behat_wwwroot. It sounds that the check should be run after the set. Or if that's not the case, because empties are used by the checker... then clarify that somehow. Although, the most I think about it, the most I'm convinced that the variable should be set before and then the checker handle it (instead of empties). Plz, consider it.

            2) To backport or not to backport? This is clearly an improvement so shouldn't be backported. But if BC is guaranteed (old configs continue working 100% the same), surely for commodity of all the testers around the globe, this could be considered. Opinions welcome.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - It all looks ok, the only two points I'm not 100% sure are: 1) In admin/tool/behat/cli/util.php, it seems logically incorrect to call behat_check_config_vars() and later , set $CFG->behat_wwwroot. It sounds that the check should be run after the set. Or if that's not the case, because empties are used by the checker... then clarify that somehow. Although, the most I think about it, the most I'm convinced that the variable should be set before and then the checker handle it (instead of empties). Plz, consider it. 2) To backport or not to backport? This is clearly an improvement so shouldn't be backported. But if BC is guaranteed (old configs continue working 100% the same), surely for commodity of all the testers around the globe, this could be considered. Opinions welcome.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            It has been decided it's ok to backport this is BC is guaranteed. Please, take a look to point 1 and feel free to move to integration, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - It has been decided it's ok to backport this is BC is guaranteed. Please, take a look to point 1 and feel free to move to integration, thanks!
            Hide
            dmonllao David Monllaó added a comment -

            Pull branches updated. Thanks for the review.

            Show
            dmonllao David Monllaó added a comment - Pull branches updated. Thanks for the review.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks David this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks David this has been integrated now.
            Hide
            fred Frédéric Massart added a comment - - edited

            This test should be failed because of this: MDL-43185. David pointed out to me that I should not have created a new issue for a regression currently in integration. My bad, I first assumed that this issue was closed, and realised it wasn't. Feel free to close the related issue, though the patch there could help.

            Show
            fred Frédéric Massart added a comment - - edited This test should be failed because of this: MDL-43185 . David pointed out to me that I should not have created a new issue for a regression currently in integration. My bad, I first assumed that this issue was closed, and realised it wasn't. Feel free to close the related issue, though the patch there could help.
            Hide
            dmonllao David Monllaó added a comment -

            Many thanks fred, please Sam, can you pull Fred's patch?

            Show
            dmonllao David Monllaó added a comment - Many thanks fred, please Sam, can you pull Fred's patch?
            Hide
            dmonllao David Monllaó added a comment - - edited

            Hi Sam, Mark Nelson reported a failed phpunit test in the dev chat, it is caused by this issue:

            vidm@davidm-hq:~/Desktop/moodlecode/INTEGRATION/master$ vendor/bin/phpunit admin/tool/behat/tests/tool_behat_test.php  --verbose
            Moodle 2.7dev (Build: 20131129), pgsql, 5ba212b4445d3e825fefafd8942fc755d969b84c
            PHPUnit 3.7.28 by Sebastian Bergmann.
             
            Configuration read from /home/davidm/Desktop/moodlecode/INTEGRATION/master/phpunit.xml
             
            .E
             
            Time: 1.45 seconds, Memory: 53.50Mb
             
            There was 1 error:
             
            1) tool_behat_testcase::test_config_file_contents
            Undefined property: stdClass::$behat_wwwroot
             
            /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/behat/classes/behat_config_manager.php:193
            /home/davidm/Desktop/moodlecode/INTEGRATION/master/admin/tool/behat/tests/tool_behat_test.php:60
            /home/davidm/Desktop/moodlecode/INTEGRATION/master/admin/tool/behat/tests/tool_behat_test.php:173
            /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/phpunit/classes/advanced_testcase.php:80
             
            To re-run:
             vendor/bin/phpunit --verbose tool_behat_testcase admin/tool/behat/tests/tool_behat_test.php
             
            FAILURES!
            Tests: 2, Assertions: 12, Errors: 1.
            

            It is not always failing, depends on the CFG->behat_* settings. Attaching patch (on top of integration) with a fix for it.

            git pull git://github.com/dmonllao/moodle.git MDL-41592-unittest-fix

            Show
            dmonllao David Monllaó added a comment - - edited Hi Sam, Mark Nelson reported a failed phpunit test in the dev chat, it is caused by this issue: vidm@davidm-hq:~/Desktop/moodlecode/INTEGRATION/master$ vendor/bin/phpunit admin/tool/behat/tests/tool_behat_test.php --verbose Moodle 2.7dev (Build: 20131129), pgsql, 5ba212b4445d3e825fefafd8942fc755d969b84c PHPUnit 3.7.28 by Sebastian Bergmann.   Configuration read from /home/davidm/Desktop/moodlecode/INTEGRATION/master/phpunit.xml   .E   Time: 1.45 seconds, Memory: 53.50Mb   There was 1 error:   1) tool_behat_testcase::test_config_file_contents Undefined property: stdClass::$behat_wwwroot   /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/behat/classes/behat_config_manager.php:193 /home/davidm/Desktop/moodlecode/INTEGRATION/master/admin/tool/behat/tests/tool_behat_test.php:60 /home/davidm/Desktop/moodlecode/INTEGRATION/master/admin/tool/behat/tests/tool_behat_test.php:173 /home/davidm/Desktop/moodlecode/INTEGRATION/master/lib/phpunit/classes/advanced_testcase.php:80   To re-run: vendor/bin/phpunit --verbose tool_behat_testcase admin/tool/behat/tests/tool_behat_test.php   FAILURES! Tests: 2, Assertions: 12, Errors: 1. It is not always failing, depends on the CFG->behat_* settings. Attaching patch (on top of integration) with a fix for it. git pull git://github.com/dmonllao/moodle.git MDL-41592 -unittest-fix
            Hide
            skodak Petr Skoda added a comment -

            works for me, and thanks a lot for the extra explanation

            Show
            skodak Petr Skoda added a comment - works for me, and thanks a lot for the extra explanation
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            NOTE:

            • Neither "git pull git://github.com/dmonllao/moodle.git MDL-41592-unittest-fix" nor MDL-43185 have been applied.
            • Looking at the 1st... are you sure that the solution is to hack the test? Shouldn't any wrong situation be handled @ behat_config_manager.php? Note I'm not sure, just guessing.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited NOTE: Neither "git pull git://github.com/dmonllao/moodle.git MDL-41592 -unittest-fix" nor MDL-43185 have been applied. Looking at the 1st... are you sure that the solution is to hack the test? Shouldn't any wrong situation be handled @ behat_config_manager.php? Note I'm not sure, just guessing. Ciao
            Hide
            samhemelryk Sam Hemelryk added a comment -

            OK Fred's patch looked good - thats being pulled now.
            Moving onto the second.

            Show
            samhemelryk Sam Hemelryk added a comment - OK Fred's patch looked good - thats being pulled now. Moving onto the second.
            Hide
            dmonllao David Monllaó added a comment -

            About the second patch, the one that fixes the unit test. $CFG->behat_wwwroot must always be set when using the behat site or running it's utilities so IMO is ok to hack the test code as we should reproduce the environment where the method will be executed.

            Show
            dmonllao David Monllaó added a comment - About the second patch, the one that fixes the unit test. $CFG->behat_wwwroot must always be set when using the behat site or running it's utilities so IMO is ok to hack the test code as we should reproduce the environment where the method will be executed.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for clarifying David. I've cherry-picked your fix now. Please be careful in the future you used the wrong bug number in your commit

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for clarifying David. I've cherry-picked your fix now. Please be careful in the future you used the wrong bug number in your commit
            Hide
            dmonllao David Monllaó added a comment -

            Grr, thanks Sam, I tend to prefix with MDL-42* these last weeks

            Show
            dmonllao David Monllaó added a comment - Grr, thanks Sam, I tend to prefix with MDL-42 * these last weeks
            Hide
            dmonllao David Monllaó added a comment -

            You can grrrr me... Jerome found another case there behat_config_manager::get_config_file_contents() needs $CFG->behat_wwwroot to be set and it is not currently being set, when accessing the list of features from the production site, there is a notice caused by this issue in latest integration. I've look for other uses of CFG->behat_wwwroot without being set and I found nothing, but we should delegate the wrong situation to get_config_file_contents()

            git pull git://github.com/dmonllao/moodle.git MDL-41592-fix-notice

            The unit test fix (a9c32ee29ea0dffab9632e406a1250f54077048c) can be reverted if you want.

            Show
            dmonllao David Monllaó added a comment - You can grrrr me... Jerome found another case there behat_config_manager::get_config_file_contents() needs $CFG->behat_wwwroot to be set and it is not currently being set, when accessing the list of features from the production site, there is a notice caused by this issue in latest integration. I've look for other uses of CFG->behat_wwwroot without being set and I found nothing, but we should delegate the wrong situation to get_config_file_contents() git pull git://github.com/dmonllao/moodle.git MDL-41592 -fix-notice The unit test fix (a9c32ee29ea0dffab9632e406a1250f54077048c) can be reverted if you want.
            Hide
            poltawski Dan Poltawski added a comment -

            Thanks for your contributions, this change is now upstream!

            “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

            Show
            poltawski Dan Poltawski added a comment - Thanks for your contributions, this change is now upstream! “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            David, plz, move on to another issue any pending issue with this issue. LOL!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - David, plz, move on to another issue any pending issue with this issue. LOL!
            Hide
            dmonllao David Monllaó added a comment -

            Thanks Eloy, done: MDL-43235

            Show
            dmonllao David Monllaó added a comment - Thanks Eloy, done: MDL-43235
            Hide
            dmonllao David Monllaó added a comment -

            Removing dev_docs_required as docs were updated in MDL-43213

            Show
            dmonllao David Monllaó added a comment - Removing dev_docs_required as docs were updated in MDL-43213

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14