Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_admin_apply_default_settings failed asserting that two arrays are identical. #723

Open
ctam opened this issue May 15, 2024 · 2 comments

Comments

@ctam
Copy link

ctam commented May 15, 2024

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
Copy link
Author

ctam commented Jun 28, 2024

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.

ctam added a commit to ctam/moodle-mod_turnitintooltwo that referenced this issue Jul 3, 2024
ctam added a commit to ctam/moodle-mod_turnitintooltwo that referenced this issue Jul 4, 2024
ctam added a commit to ctam/moodle-mod_turnitintooltwo that referenced this issue Jul 4, 2024
@gbarat87
Copy link

Hi, replacing 0 by 'https://api.turnitin.com' will just update your unit test failure to

-Array &0 ()
+Array &0 (
+    'turnitintooltwo/apiurl' => 'https://api.turnitin.com'
+)

The default settings should be update to null

lbailey-ucsf added a commit to ucsf-education/moodle-mod_turnitintooltwo that referenced this issue Nov 13, 2024
…null instead of 0. This change aims to address issue turnitin#723 in the upstream repositoty.
gbarat87 added a commit to gbarat87/moodle-mod_turnitintooltwo that referenced this issue Nov 15, 2024
dmitriim pushed a commit to catalyst/moodle-mod_turnitintooltwo that referenced this issue Nov 19, 2024
dmitriim added a commit to catalyst/moodle-mod_turnitintooltwo that referenced this issue Nov 19, 2024
fix turnitin#779 fix turnitin#723 update default value for settings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants