turnitin / moodle-mod_turnitintooltwo

Turnitin Direct plugin (version 2) for Moodle
http://www.turnitin.com
32 stars 63 forks source link

test_admin_apply_default_settings failed asserting that two arrays are identical. #723

Open ctam opened 1 month ago

ctam commented 1 month ago

In Moodle 4.3, this plugin (v2024032901) causes a core unit test failure as follows:

Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 ()
+Array &0 (
+    'turnitintooltwo/apiurl' => '0'
+)

/home/runner/work/moodle/moodle/lib/tests/adminlib_test.php:173
/home/runner/work/moodle/moodle/lib/phpunit/classes/advanced_testcase.php:81
ctam commented 2 days ago

I think I find the issue here.

In settings.php, the $testoptions array is using strings as its key instead of number:

    $testoptions = array(
        'https://api.turnitin.com' => 'https://api.turnitin.com',
        'https://api.turnitinuk.com' => 'https://api.turnitinuk.com',
        'https://sandbox.turnitin.com' => 'https://sandbox.turnitin.com'
    );

And later in this code, it uses a numeric value to set the default value (the 4th argument), instead of a string:

    $settings->add(new admin_setting_configselect('turnitintooltwo/apiurl',
                                    get_string("turnitinapiurl", "turnitintooltwo"),
                                    get_string("turnitinapiurl_desc", "turnitintooltwo").$offlinecomment.$testconnection,
                                    0, $testoptions));

I believe, this can be fixed by setting the default to the first value, 'https://api.turnitin.com' in the array.

    $settings->add(new admin_setting_configselect('turnitintooltwo/apiurl',
                                    get_string("turnitinapiurl", "turnitintooltwo"),
                                    get_string("turnitinapiurl_desc", "turnitintooltwo").$offlinecomment.$testconnection,
                                    'https://api.turnitin.com', $testoptions));

But better yet, I think these could be refactored to use some constants for these URLs, and have this array changed to numeric indexes.