Details

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

      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

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Removing blocker link on MDL-32362

          Show
          Dan Poltawski added a comment - Removing blocker link on MDL-32362
          Hide
          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
          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
          Dan Poltawski added a comment -

          Passing, thanks

          Show
          Dan Poltawski added a comment - Passing, thanks
          Hide
          Petr Škoda 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
          Petr Škoda 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
          Dan Poltawski added a comment -

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

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

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

          Show
          Dan Poltawski added a comment - Ah of course, my problem was that I put testdataroot in same location as dataroot (web access)
          Hide
          Petr Škoda 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
          Petr Škoda 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
          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
          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
          Petr Škoda added a comment -

          ah, thanks for the info

          Show
          Petr Škoda added a comment - ah, thanks for the info
          Hide
          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
          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
          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
          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
          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
          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: