Details

    • Type: Task
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Unit tests
    • Labels:

      Description

      converted and new tests:

      • gradebook tests
      • phpunit integration self-tests
      • /question/* - 2 minor issues left - see MDL-32375
      • mod_data search

      other:

      • wrapper for execution via Web UI - for OU test servers (for now hidden) - admin/tool/phpunit/webrunner.php
      • automated initialisation shell script in admin/tool/phpunit/cli/init.*
      • custom db connection info via $CFG->phpunit_database ...
      • better diagnostics of missing $CFG->phpunit_dataroot and $CFG->phpunit_prefix
      • new module and block callbacks in data generator
      • better perf for mysql and pg, more accurate reset after each test
      • Windows supported
      • schema for phpunit.xml configuration
      • prevention of cuncurrent test execution
      • support for global $DB mocking
      • dataset loading support
      • many more bugfixes and improvements

      documentation:

      todo in stage 4:

      • discuss problem in navigationlib_test.php method test_find_expandable() with SamH
      • completionlib - using mocks heavily
      • webservices - seems to do nothing
      • delete all converted simpletests
      • hide simpletest UI

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            poltawski Dan Poltawski added a comment -

            Removing blocker link on MDL-32362

            Show
            poltawski Dan Poltawski added a comment - Removing blocker link on MDL-32362
            Hide
            poltawski Dan Poltawski added a comment -

            Hi Petr,

            I have integrated this now, however I have some comments that I think also should be considered for stage 4.

            1. phpunit_bootstrap_error(130); I think that we have these raw error code numbers in enough places in Moodle code that they should be constants - we have 25 of them around and I don't understand why you went for using the raw numbers? Is it to match the phpunit error code? I think these would be more self-documenting as constants.
            2. Following from that, these two should should present two different error messages:

              // verify PHPUnit libs are loaded
              if (!@include_once('PHPUnit/Autoload.php')) {
                  phpunit_bootstrap_error(130);
              }
               
              if (!@include_once('PHPUnit/Extensions/Database/Autoload.php')) {
                  phpunit_bootstrap_error(130);
              }

              I was greeted with 'Moodle can not find PHPUnit PEAR library or necessary PHPUnit extension' on my OSX install. Yet I could run the phpunit command fine - turns out the database extension wasn't automatically installed with phpunit, so I had to do it. I had to dig into the code to find out what was wrong - this should also be in the installation instructions on the wiki (notice you've added to the text file). Perhaps we shouldn't bother hididng the errors - it might be more self documenting.

              pear install phpunit/DbUnit

            3. Moodle can not create PHPUnit configuration file, please verify dirroot permissions. Generally people have permissions set up so that the web users can't modify dirroot but can modify data root. It seems the requirement here conflicts because I couldn't run as my normal user (as it needed access to dataroot) or the web user (as needed access to dirroot).
            4. Note that the web runner currently seems to be affected by this permissions problem too despite using data root to write webrunner.xml. When I first tested this that was working and using that file but its no longer doing that. Perhaps if we lack dirroot permissions we could create the file in dataroot and give the developer the command to copy the file from there into dirroot.
            Show
            poltawski Dan Poltawski added a comment - Hi Petr, I have integrated this now, however I have some comments that I think also should be considered for stage 4. phpunit_bootstrap_error(130); I think that we have these raw error code numbers in enough places in Moodle code that they should be constants - we have 25 of them around and I don't understand why you went for using the raw numbers? Is it to match the phpunit error code? I think these would be more self-documenting as constants. Following from that, these two should should present two different error messages: // verify PHPUnit libs are loaded if (!@include_once('PHPUnit/Autoload.php')) { phpunit_bootstrap_error(130); }   if (!@include_once('PHPUnit/Extensions/Database/Autoload.php')) { phpunit_bootstrap_error(130); } I was greeted with 'Moodle can not find PHPUnit PEAR library or necessary PHPUnit extension' on my OSX install. Yet I could run the phpunit command fine - turns out the database extension wasn't automatically installed with phpunit, so I had to do it. I had to dig into the code to find out what was wrong - this should also be in the installation instructions on the wiki (notice you've added to the text file). Perhaps we shouldn't bother hididng the errors - it might be more self documenting. pear install phpunit/DbUnit Moodle can not create PHPUnit configuration file, please verify dirroot permissions. Generally people have permissions set up so that the web users can't modify dirroot but can modify data root. It seems the requirement here conflicts because I couldn't run as my normal user (as it needed access to dataroot) or the web user (as needed access to dirroot). Note that the web runner currently seems to be affected by this permissions problem too despite using data root to write webrunner.xml. When I first tested this that was working and using that file but its no longer doing that. Perhaps if we lack dirroot permissions we could create the file in dataroot and give the developer the command to copy the file from there into dirroot.
            Hide
            poltawski Dan Poltawski added a comment -

            Passing, thanks

            Show
            poltawski Dan Poltawski added a comment - Passing, thanks
            Hide
            skodak Petr Skoda added a comment -

            1. I needed to use these numbers in init scripts so I did not think much about constants, I will deal with this next week, thanks. I invented my own error numbers >128

            2. I have removed the @, thanks

            3. it conflicts in web runner only, you should run phpunit under your developer account which should have write access both to testdataroot and dirroot - this is the main reason why I discourage use of webrunner.php

            4. I will probably add new --webrunner parameter to the util.php script and skip phpunit.xml if dirroot not writable - next week

            Show
            skodak Petr Skoda added a comment - 1. I needed to use these numbers in init scripts so I did not think much about constants, I will deal with this next week, thanks. I invented my own error numbers >128 2. I have removed the @, thanks 3. it conflicts in web runner only, you should run phpunit under your developer account which should have write access both to testdataroot and dirroot - this is the main reason why I discourage use of webrunner.php 4. I will probably add new --webrunner parameter to the util.php script and skip phpunit.xml if dirroot not writable - next week
            Hide
            poltawski Dan Poltawski added a comment -

            Ah of course, my problem was that I put testdataroot in same location as dataroot (web access)

            Show
            poltawski Dan Poltawski added a comment - Ah of course, my problem was that I put testdataroot in same location as dataroot (web access)
            Hide
            poltawski Dan Poltawski added a comment -

            Ah of course, my problem was that I put testdataroot in same location as dataroot (web access)

            Show
            poltawski Dan Poltawski added a comment - Ah of course, my problem was that I put testdataroot in same location as dataroot (web access)
            Hide
            skodak Petr Skoda added a comment -

            eh, in that case I have to make sure nobody points these two dataroots to one location because they would soon loose production data...

            Show
            skodak Petr Skoda added a comment - eh, in that case I have to make sure nobody points these two dataroots to one location because they would soon loose production data...
            Hide
            poltawski Dan Poltawski added a comment -

            Oh I didn't point them at the same location, but different subdirectories of the 'apache writeable directory

            I would not encourage anyone to use unit tests on their production data anywhere, just in case!

            Show
            poltawski Dan Poltawski added a comment - Oh I didn't point them at the same location, but different subdirectories of the 'apache writeable directory I would not encourage anyone to use unit tests on their production data anywhere, just in case!
            Hide
            skodak Petr Skoda added a comment -

            ah, thanks for the info

            Show
            skodak Petr Skoda added a comment - ah, thanks for the info
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Some extra commits has been added related with this after testing all the stuff against mssql (me) and sqlsrv (petr):

            • Changes to the sqlsrv driver in order to avoid a lot of superfluous logging output.
            • Changes to the mssql generator (shared by both the mssql and the sqlsrv drivers) to fix one problem reseting sequences. Plus extra tests to cover the case.
            • Changes to tests, to fix some of them broken.
            • Changes to tests, to work under case sensitive mssql and sqlsrv sites.
            • Improvement to the phpunit execution so it now can use rollback for big speed gains on reset.

            Testing everything right now...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Some extra commits has been added related with this after testing all the stuff against mssql (me) and sqlsrv (petr): Changes to the sqlsrv driver in order to avoid a lot of superfluous logging output. Changes to the mssql generator (shared by both the mssql and the sqlsrv drivers) to fix one problem reseting sequences. Plus extra tests to cover the case. Changes to tests, to fix some of them broken. Changes to tests, to work under case sensitive mssql and sqlsrv sites. Improvement to the phpunit execution so it now can use rollback for big speed gains on reset. Testing everything right now...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -
            • simpletest against the 4 DBs passing ok.
            • php unit execution against mysql, pgsql, mssql and sqlsrv passing ok.

            So all ok, big thanks, Petr, ciao

            PS: Pushing right now.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - simpletest against the 4 DBs passing ok. php unit execution against mysql, pgsql, mssql and sqlsrv passing ok. So all ok, big thanks, Petr, ciao PS: Pushing right now.
            Hide
            poltawski Dan Poltawski added a comment -

            Jolly good show!

            Your changes have made it into the Moodle release - its time to celebrate! I suggest a hot cup of English tea (with milk, no sugar) or a hoppy English ale.

            Tally-ho!

            Show
            poltawski Dan Poltawski added a comment - Jolly good show! Your changes have made it into the Moodle release - its time to celebrate! I suggest a hot cup of English tea (with milk, no sugar) or a hoppy English ale. Tally-ho!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  25/Jun/12