Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2
    • Fix Version/s: 2.3
    • Component/s: Libraries
    • Labels:
      None
    • Rank:
      39485

      Issue Links

        Activity

        Hide
        Dan Poltawski added a comment -

        Hi Petr,

        I've integrated this now.

        I think that there is use in testing auth/db/ too since there are no unit tests there. Also a manual test of enrol/db probably worth a go since these are the only two core pieces which use it - if you can expand the test instructions for that would be good.

        Show
        Dan Poltawski added a comment - Hi Petr, I've integrated this now. I think that there is use in testing auth/db/ too since there are no unit tests there. Also a manual test of enrol/db probably worth a go since these are the only two core pieces which use it - if you can expand the test instructions for that would be good.
        Hide
        Petr Škoda added a comment -

        Hi, I am afraid we can not test all settings ppl use for addodb in auth end enrol plugins, that is why I tried hard to create new unit test at least in enrol plugin, the auth plugin will be partially rewritten in 2.4 that is why I did not test it too.

        Testers: please use auth and enrol db docs if necessary, and improve them if you find any confusion there, thanks.

        Show
        Petr Škoda added a comment - Hi, I am afraid we can not test all settings ppl use for addodb in auth end enrol plugins, that is why I tried hard to create new unit test at least in enrol plugin, the auth plugin will be partially rewritten in 2.4 that is why I did not test it too. Testers: please use auth and enrol db docs if necessary, and improve them if you find any confusion there, thanks.
        Hide
        Michael de Raadt added a comment -

        Dan has volunteered to run the external DB tests and I will run the phpunit tests.

        Show
        Michael de Raadt added a comment - Dan has volunteered to run the external DB tests and I will run the phpunit tests.
        Hide
        Michael de Raadt added a comment -

        Test result: Not successful.

        I was running the PHPUnit tests on all DBs and during the SQL Server run the following error presented itself...

        .............................................................  671 / 1158 ( 57%)
        
        ...............................................Missing file: D:\xampp\htdocs\moo
        dle_testing_mssql\lib\adodb/drivers/adodb-sqlsrv.inc.php
        <p>ADONewConnection: Unable to load database driver ''</p>
        Fatal error: Call to a member function IsConnected() on a non-object in D:\xampp
        \htdocs\moodle_testing_mssql\enrol\database\lib.php on line 788
        

        It looks like the test is looking for the wrong file, but neither Dan or myself know enough about it.

        Show
        Michael de Raadt added a comment - Test result: Not successful. I was running the PHPUnit tests on all DBs and during the SQL Server run the following error presented itself... ............................................................. 671 / 1158 ( 57%) ...............................................Missing file: D:\xampp\htdocs\moo dle_testing_mssql\lib\adodb/drivers/adodb-sqlsrv.inc.php <p>ADONewConnection: Unable to load database driver ''</p> Fatal error: Call to a member function IsConnected() on a non-object in D:\xampp \htdocs\moodle_testing_mssql\enrol\database\lib.php on line 788 It looks like the test is looking for the wrong file, but neither Dan or myself know enough about it.
        Hide
        Petr Škoda added a comment -

        Problem fixed, please ignore error log output from the sqlsrv driver during testing, I do not know yet how to resolve that.

        Show
        Petr Škoda added a comment - Problem fixed, please ignore error log output from the sqlsrv driver during testing, I do not know yet how to resolve that.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        (new commit with fix applied, reseting to testing status, thanks!)

        Show
        Eloy Lafuente (stronk7) added a comment - (new commit with fix applied, reseting to testing status, thanks!)
        Hide
        Michael de Raadt added a comment -

        I just finished the PHPUnit tests under Oracle and the following error appeared.

        There was 1 error:
        
        1) core_adodb_testcase::test_read_table
        Undefined index: id
        
        D:\xampp\htdocs\moodle_testing_oracle\enrol\database\tests\adodb_test.php:98
        D:\xampp\htdocs\moodle_testing_oracle\lib\phpunit\lib.php:1162
        D:\xampp\php\phpunit:46
        

        I'm grabbing your latest changes and I'll run that test again specifically.

        Show
        Michael de Raadt added a comment - I just finished the PHPUnit tests under Oracle and the following error appeared. There was 1 error: 1) core_adodb_testcase::test_read_table Undefined index: id D:\xampp\htdocs\moodle_testing_oracle\enrol\database\tests\adodb_test.php:98 D:\xampp\htdocs\moodle_testing_oracle\lib\phpunit\lib.php:1162 D:\xampp\php\phpunit:46 I'm grabbing your latest changes and I'll run that test again specifically.
        Hide
        Michael de Raadt added a comment -

        After grabbing the latest integration changes the same error appeared under Oracle.

        I'll try again SQL Server now.

        Show
        Michael de Raadt added a comment - After grabbing the latest integration changes the same error appeared under Oracle. I'll try again SQL Server now.
        Hide
        Michael de Raadt added a comment -

        The PHPUnit tests now run and complete under SQL Server now. Apart from a case-related fail, all tests pass.

        I can't see Petr online at the moment. I will check this issue again on Thursday.

        Show
        Michael de Raadt added a comment - The PHPUnit tests now run and complete under SQL Server now. Apart from a case-related fail, all tests pass. I can't see Petr online at the moment. I will check this issue again on Thursday.
        Hide
        Michael de Raadt added a comment -

        Sorry, I should have failed this on Tuesday. I was hoping Petr would have a chance to look at this again and see what could be done about the Oracle fail.

        Show
        Michael de Raadt added a comment - Sorry, I should have failed this on Tuesday. I was hoping Petr would have a chance to look at this again and see what could be done about the Oracle fail.
        Hide
        Petr Škoda added a comment -

        I will try to install and fix the oracle today, in any case it should not be a reason to rollback this issue if it does not get fixed because the other db tests work...

        Show
        Petr Škoda added a comment - I will try to install and fix the oracle today, in any case it should not be a reason to rollback this issue if it does not get fixed because the other db tests work...
        Hide
        Petr Škoda added a comment -

        Ah, the missing 'id' is caused by the fact that oracle returns capital letter 'ID', this means that this new test most probably discovered existing problem. Please note I never tested anything enrol related with Oracle before.

        Show
        Petr Škoda added a comment - Ah, the missing 'id' is caused by the fact that oracle returns capital letter 'ID', this means that this new test most probably discovered existing problem. Please note I never tested anything enrol related with Oracle before.
        Hide
        Petr Škoda added a comment -

        I have created new issue for the oracle trouble - see MDL-32645, please consider approving this as tested.

        Show
        Petr Škoda added a comment - I have created new issue for the oracle trouble - see MDL-32645 , please consider approving this as tested.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        yeah, adodb's driver (oracle) returns table/column names uppercase by default. We had that under control for Moodle 1.x with preconfigure_dbconnection() if I'm not wrong, by setting ADODB_ASSOC_CASE.

        Show
        Eloy Lafuente (stronk7) added a comment - yeah, adodb's driver (oracle) returns table/column names uppercase by default. We had that under control for Moodle 1.x with preconfigure_dbconnection() if I'm not wrong, by setting ADODB_ASSOC_CASE.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Reseting to "testable" status. For your consideration, dear tester.

        Show
        Eloy Lafuente (stronk7) added a comment - Reseting to "testable" status. For your consideration, dear tester.
        Hide
        Michael de Raadt added a comment -

        Test result: Success

        Apart from the reported index error, covered in the linked issue, this passes.

        Show
        Michael de Raadt added a comment - Test result: Success Apart from the reported index error, covered in the linked issue, this passes.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        This has been near becoming rejected, because it's not the best code you are able to produce.

        But, luckily, at the end, it has landed and has been spread to all repos out there.

        Many thanks and, don't forget it, keep improving your skills, you can!

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - This has been near becoming rejected, because it's not the best code you are able to produce. But, luckily, at the end, it has landed and has been spread to all repos out there. Many thanks and, don't forget it, keep improving your skills, you can! Closing, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: