Details

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

      Gliffy Diagrams

        Attachments

          Issue Links

            Activity

            Hide
            poltawski 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
            poltawski 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            salvetore Michael de Raadt added a comment -

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

            Show
            salvetore Michael de Raadt added a comment - Dan has volunteered to run the external DB tests and I will run the phpunit tests.
            Hide
            salvetore 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
            salvetore 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - (new commit with fix applied, reseting to testing status, thanks!)
            Hide
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            salvetore 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda 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
            skodak Petr Skoda added a comment -

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

            Show
            skodak Petr Skoda added a comment - I have created new issue for the oracle trouble - see MDL-32645 , please consider approving this as tested.
            Hide
            stronk7 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
            stronk7 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

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

            Test result: Success

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

            Show
            salvetore Michael de Raadt added a comment - Test result: Success Apart from the reported index error, covered in the linked issue, this passes.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  25/Jun/12