MDL-79717 phpunit: ensure unique data provider keys in tests.

Duplicate data provider keys were overwriting and/or duplicating one another, leading to some cases being skipped. Other "duplicate array key" errors were picked up by `phpcs` in this dragnet across all tests, which have also been fixed.
marxjohnson · Jul 10, 2024 · bd0f8a0 · bd0f8a0
1 parent 7d7a871
commit bd0f8a0
Show file tree

Hide file tree

Showing 17 changed files with 75 additions and 162 deletions.
diff --git a/backup/moodle2/tests/moodle2_test.php b/backup/moodle2/tests/moodle2_test.php
@@ -36,7 +36,7 @@
  * @copyright 2014 The Open University
  * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class moodle2_test extends \advanced_testcase {
+final class moodle2_test extends \advanced_testcase {
 
     /**
      * Tests the availability field on modules and sections is correctly
@@ -1146,7 +1146,6 @@ public function test_xapistate_backup(): void {
             'filearea' => 'package',
             'itemid' => 0,
             'filepath' => '/',
-            'filepath' => '/',
             'filename' => 'dummy.h5p',
             'addxapistate' => true,
         ];

diff --git a/course/tests/externallib_test.php b/course/tests/externallib_test.php
@@ -39,8 +39,7 @@
  * @copyright  2012 Jerome Mouneyrac
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class externallib_test extends externallib_advanced_testcase {
-    //core_course_externallib_testcase
+final class externallib_test extends externallib_advanced_testcase {
 
     /**
      * Tests set up
@@ -609,7 +608,7 @@ public function test_create_courses(): void {
      *
      * @return array
      */
-    public function course_empty_field_provider(): array {
+    public static function course_empty_field_provider(): array {
         return [
             [[
                 'fullname' => '',
@@ -3304,7 +3303,7 @@ public function test_check_updates(): void {
     /**
      * Test cases for the get_enrolled_courses_by_timeline_classification test.
      */
-    public function get_get_enrolled_courses_by_timeline_classification_test_cases(): array {
+    public static function get_get_enrolled_courses_by_timeline_classification_test_cases(): array {
         $now = time();
         $day = 86400;
 
@@ -3561,16 +3560,6 @@ public function get_get_enrolled_courses_by_timeline_classification_test_cases()
                 'expectednextoffset' => 0,
                 'expectedexception' => 'Invalid $sort parameter in enrol_get_my_courses()',
             ],
-            'all limit and offset with wrong sort direction' => [
-                'coursedata' => $coursedata,
-                'classification' => 'all',
-                'limit' => 5,
-                'offset' => 5,
-                'sort' => "ul.timeaccess abcdasc",
-                'expectedcourses' => [],
-                'expectednextoffset' => 0,
-                'expectedexception' => 'Invalid sort direction in $sort parameter in enrol_get_my_courses()',
-            ],
             'all limit and offset with wrong sort direction' => [
                 'coursedata' => $coursedata,
                 'classification' => 'all',
@@ -3591,16 +3580,6 @@ public function get_get_enrolled_courses_by_timeline_classification_test_cases()
                 'expectednextoffset' => 0,
                 'expectedexception' => 'Invalid $sort parameter in enrol_get_my_courses()',
             ],
-            'all limit and offset with wrong field name' => [
-                'coursedata' => $coursedata,
-                'classification' => 'all',
-                'limit' => 5,
-                'offset' => 5,
-                'sort' => "ul.foobar",
-                'expectedcourses' => [],
-                'expectednextoffset' => 0,
-                'expectedexception' => 'Invalid $sort parameter in enrol_get_my_courses()',
-            ],
             'all limit and offset with wrong field separator' => [
                 'coursedata' => $coursedata,
                 'classification' => 'all',

diff --git a/enrol/lti/tests/local/ltiadvantage/entity/user_test.php b/enrol/lti/tests/local/ltiadvantage/entity/user_test.php
@@ -24,7 +24,7 @@
  * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  * @coversDefaultClass \enrol_lti\local\ltiadvantage\entity\user
  */
-class user_test extends \advanced_testcase {
+final class user_test extends \advanced_testcase {
 
     /**
      * Test creation of a user instance using the factory method.
@@ -64,7 +64,7 @@ public function test_create(array $args, array $expectations): void {
      *
      * @return array the data for testing.
      */
-    public function create_data_provider(): array {
+    public static function create_data_provider(): array {
         global $CFG;
         return [
             'Valid create, only required args provided' => [
@@ -272,7 +272,7 @@ public function test_create_from_resource_link(array $args, array $expectations)
      *
      * @return array the data for testing.
      */
-    public function create_from_resource_link_data_provider(): array {
+    public static function create_from_resource_link_data_provider(): array {
         global $CFG;
         return [
             'Valid creation, all args provided explicitly' => [
@@ -511,7 +511,7 @@ public function test_setters_and_getters(string $methodname, $arg, array $expect
      *
      * @return array the array of test data.
      */
-    public function setters_getters_data_provider(): array {
+    public static function setters_getters_data_provider(): array {
         global $CFG;
         return [
             'Testing set_resourcelinkid with valid id' => [
@@ -620,12 +620,12 @@ public function setters_getters_data_provider(): array {
             ],
             'Testing set_maildisplay with a valid int: 2' => [
                 'methodname' => 'maildisplay',
-                'arg' => '1',
+                'arg' => '2',
                 'expectations' => [
                     'valid' => true,
                 ]
             ],
-            'Testing set_maildisplay with invalid int' => [
+            'Testing set_maildisplay with invalid int: -1' => [
                 'methodname' => 'maildisplay',
                 'arg' => '-1',
                 'expectations' => [
@@ -634,7 +634,7 @@ public function setters_getters_data_provider(): array {
                     'exceptionmessage' => "Invalid maildisplay value '-1'. Must be in the range {0..2}."
                 ]
             ],
-            'Testing set_maildisplay with invalid int' => [
+            'Testing set_maildisplay with invalid int: 3' => [
                 'methodname' => 'maildisplay',
                 'arg' => '3',
                 'expectations' => [
@@ -659,7 +659,7 @@ public function setters_getters_data_provider(): array {
                     'exceptionmessage' => 'Invalid lang value. Cannot be an empty string.'
                 ]
             ],
-            'Testing set_lang with an empty string' => [
+            'Testing set_lang with invalid lang code' => [
                 'methodname' => 'lang',
                 'arg' => 'ff',
                 'expectations' => [

diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php
@@ -38,12 +38,12 @@
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  * @since Moodle 2.4
  */
-class externallib_test extends externallib_advanced_testcase {
+final class externallib_test extends externallib_advanced_testcase {
 
     /**
      * dataProvider for test_get_enrolled_users_visibility().
      */
-    public function get_enrolled_users_visibility_provider() {
+    public static function get_enrolled_users_visibility_provider(): array {
         return array(
             'Course without groups, default behavior (not filtering by cap, group, active)' =>
             array(
@@ -232,7 +232,7 @@ public function get_enrolled_users_visibility_provider() {
                 ),
             ),
 
-            'Course with separate groups, filtering by withcapability (having moodle/role:review)' =>
+            'Course with separate groups, filtering by withcapability (having moodle/role:review & moodle/course:bulkmessaging)' =>
             array(
                 'settings' => array(
                     'coursegroupmode' => SEPARATEGROUPS,
@@ -1125,7 +1125,7 @@ public function test_get_enrolled_users_with_capability_including_lastcourseacce
     /**
      * dataProvider for test_submit_user_enrolment_form().
      */
-    public function submit_user_enrolment_form_provider() {
+    public static function submit_user_enrolment_form_provider(): array {
         $now = new \DateTime();
 
         $nextmonth = clone($now);

diff --git a/grade/report/history/tests/report_test.php b/grade/report/history/tests/report_test.php
@@ -25,7 +25,7 @@
  * @copyright  2014 Frédéric Massart - FMCorz.net
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class report_test extends \advanced_testcase {
+final class report_test extends \advanced_testcase {
 
     /**
      * Create some grades.
@@ -251,7 +251,7 @@ public function test_get_users(): void {
      *
      * @return array List of data sets (test cases)
      */
-    public function get_users_with_profile_fields_provider(): array {
+    public static function get_users_with_profile_fields_provider(): array {
         return [
             // User identity check boxes, 'email', 'profile_field_lang' and 'profile_field_height' are checked.
                 'show email,lang and height;search for all users' =>
@@ -264,24 +264,18 @@ public function get_users_with_profile_fields_provider(): array {
                         ['email,profile_field_lang,profile_field_height', '.uk', ['u3']],
                 'show email,lang and height,search for Spanish speakers' =>
                         ['email,profile_field_lang,profile_field_height', 'spanish', ['u1', 'u4']],
-                'show email,lang and height,search for Spanish speakers' =>
+                'show email,lang and height,search for Spanish speakers (using spa)' =>
                         ['email,profile_field_lang,profile_field_height', 'spa', ['u1', 'u4']],
                 'show email,lang and height,search for German speakers' =>
                         ['email,profile_field_lang,profile_field_height', 'german', ['u2']],
-                'show email,lang and height,search for German speakers' =>
+                'show email,lang and height,search for German speakers (using ger)' =>
                         ['email,profile_field_lang,profile_field_height', 'ger', ['u2']],
                 'show email,lang and height,search for English speakers' =>
                         ['email,profile_field_lang,profile_field_height', 'english', ['u3']],
-                'show email,lang and height,search for English speakers' =>
+                'show email,lang and height,search for English speakers (using eng)' =>
                         ['email,profile_field_lang,profile_field_height', 'eng', ['u3']],
-                'show email,lang and height,search for English speakers' =>
-                        ['email,profile_field_lang,profile_field_height', 'ish', ['u3']],
-                'show email,lang and height,search for users with height 180cm' =>
-                        ['email,profile_field_lang,profile_field_height', '180cm', ['u2', 'u3', 'u4']],
                 'show email,lang and height,search for users with height 180cm' =>
                         ['email,profile_field_lang,profile_field_height', '180', ['u2', 'u3', 'u4']],
-                'show email,lang and height,search for users with height 170cm' =>
-                        ['email,profile_field_lang,profile_field_height', '170cm', ['u1']],
                 'show email,lang and height,search for users with height 170cm' =>
                         ['email,profile_field_lang,profile_field_height', '170', ['u1']],
 
@@ -292,25 +286,15 @@ public function get_users_with_profile_fields_provider(): array {
                         ['email,profile_field_height', '.com', []],
                 'show email and height;search for users on .co' =>
                         ['email,profile_field_height', '.co', ['u3']],
-                'show email and height,search for Spanish/German/English speakers' =>
+                'show email and height,search for Spanish speakers' =>
                         ['email,profile_field_height', 'spanish', []],
-                'show email and height,search for Spanish/German/English speakers' =>
+                'show email and height,search for German speakers' =>
                         ['email,profile_field_height', 'german', []],
-                'show email and height,search for Spanish/German/English speakers' =>
-                        ['email,profile_field_height', 'english', []],
-                'show email,lang and height,search for English speakers' =>
-                        ['email,profile_field_height', 'english', []],
                 'show email and height,search for English speakers' =>
-                        ['email,profile_field_height', 'eng', []],
-                'show email and height,search for English speakers' =>
-                        ['email,profile_field_height', 'ish', []],
+                        ['email,profile_field_height', 'english', []],
                 'show email and height,search for users with height 180cm' =>
-                        ['email,profile_field_height', '180cm', ['u2', 'u3', 'u4']],
-                'show email,lang and height,search for users with height 180cm' =>
                         ['email,profile_field_height', '180', ['u2', 'u3', 'u4']],
-                'show email,lang and height,search for users with height 170cm' =>
-                        ['email,profile_field_height', '170cm', ['u1']],
-                'show email,lang and height,search for users with height 170cm' =>
+                'show email and height,search for users with height 170cm' =>
                         ['email,profile_field_height', '170', ['u1']],
 
             // User identity check boxes, only 'email' is checked.
@@ -322,9 +306,7 @@ public function get_users_with_profile_fields_provider(): array {
                 'show email only;search for Spanish speakers' => ['email', 'spanish', []],
                 'show email only;search for German speakers' => ['email', 'german', []],
                 'show email only;search for English speakers' => ['email', 'english', []],
-                'show email only;search for users with height 180cm' => ['email', '180cm', []],
                 'show email only;search for users with height 180cm' => ['email', '180', []],
-                'show email only;search for users with height 170cm' => ['email', '170cm', []],
                 'show email only;search for users with height 170cm' => ['email', '170', []],
         ];
     }
@@ -401,7 +383,7 @@ public function test_get_users_with_profile_fields(string $showuseridentity, str
     /**
      * Data provider method for \gradereport_history_report_testcase::test_get_users_with_groups()
      */
-    public function get_users_provider() {
+    public static function get_users_provider(): array {
         return [
             'Visible groups, non-editing teacher, not in any group' => [
                 VISIBLEGROUPS, 'teacher', ['g1', 'g2'], ['s1', 's2', 's3', 's4', 's5']

diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php
@@ -25,7 +25,7 @@
  * @author     [email protected]
  * @author     [email protected]
  */
-class moodlelib_test extends \advanced_testcase {
+final class moodlelib_test extends \advanced_testcase {
 
     /**
      * Define a local decimal separator.
@@ -858,13 +858,15 @@ public function test_clean_param_timezone(): void {
             '-13.5'                          => '',
             '0.2'                            => '',
             ''                               => '',
-            null                             => '',
         );
 
         foreach ($testvalues as $testvalue => $expectedvalue) {
             $actualvalue = clean_param($testvalue, PARAM_TIMEZONE);
             $this->assertEquals($expectedvalue, $actualvalue);
         }
+
+        // Test for null.
+        $this->assertEquals('', clean_param(null, PARAM_TIMEZONE));
     }
 
     /**
@@ -3575,9 +3577,9 @@ public function test_setnew_password_and_mail(): void {
 
     /**
      * Data provider for test_generate_confirmation_link
-     * @return Array of confirmation urls and expected resultant confirmation links
+     * @return array Confirmation urls and expected resultant confirmation links
      */
-    public function generate_confirmation_link_provider() {
+    public static function generate_confirmation_link_provider(): array {
         global $CFG;
         return [
             "Simple name" => [
@@ -3645,7 +3647,7 @@ public function generate_confirmation_link_provider() {
                 "confirmationurl" => "http://moodle.org/ext.php?with=some&param=eters",
                 "expected" => "http://moodle.org/ext.php?with=some&param=eters&data=/many_-%2E%40characters%40_%40-%2E%2E-%2E%2E"
             ],
-            "Custom external confirmation url with parameters" => [
+            "Custom external confirmation url with parameters (again)" => [
                 "username" => "many_-.@characters@[email protected]..",
                 "confirmationurl" => "http://moodle.org/ext.php?with=some&data=test",
                 "expected" => "http://moodle.org/ext.php?with=some&data=/many_-%2E%40characters%40_%40-%2E%2E-%2E%2E"

diff --git a/mod/chat/tests/dates_test.php b/mod/chat/tests/dates_test.php
@@ -14,15 +14,6 @@
 // You should have received a copy of the GNU General Public License
 // along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
 
-/**
- * Contains unit tests for mod_chat\dates.
- *
- * @package   mod_chat
- * @category  test
- * @copyright 2021 Dongsheng Cai
- * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
- */
-
 declare(strict_types=1);
 
 namespace mod_chat;
@@ -33,8 +24,13 @@
 
 /**
  * Class for unit testing mod_chat\dates.
+ *
+ * @package   mod_chat
+ * @category  test
+ * @copyright 2021 Dongsheng Cai
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class dates_test extends advanced_testcase {
+final class dates_test extends advanced_testcase {
 
     /**
      * Setup testcase.
@@ -50,7 +46,7 @@ public function setUp(): void {
      * Data provider for get_dates_for_module().
      * @return array[]
      */
-    public function get_dates_for_module_provider(): array {
+    public static function get_dates_for_module_provider(): array {
         global $CFG;
         require_once($CFG->dirroot . '/mod/chat/lib.php');
 
@@ -62,10 +58,10 @@ public function get_dates_for_module_provider(): array {
         $weeklynextchattime = $past + 7 * DAYSECS;
         $label = get_string('nextchattime', 'mod_chat');
         return [
-            'chattime in the past' => [
+            'chattime in the past (no schedule)' => [
                 $past, CHAT_SCHEDULE_NONE, []
             ],
-            'chattime in the past' => [
+            'chattime in the past (single schedule)' => [
                 $past, CHAT_SCHEDULE_SINGLE, []
             ],
             'chattime in the future' => [