MDL-65204 phpunit: various fixes to assertions

Namely: - 3rd param of assertEquals() cannot be null. - Some incorrect uses of assertNotEmpty(). - Comparing 2 strings now uses strict (===) evaluation. Link: sebastianbergmann/phpunit#3185 Solution here is one of: a) Return to the previous situation, making the comparison softer. That can achieved by forcing different types, so float == string works. b) Changing APIs (both forms and database return strings) to perform some conversion to floats. That would make float comparison (with floats or strings) to work too. The patch here follows the a) approach. Changing all the internals for proper float handling sounds excesive when it has been working perfectly since ever. So we went the easier route, just getting rid of the new === comparisons when needed by changing expectation types to float.
mvaraujo · Apr 3, 2019 · 85f47ba · 85f47ba
1 parent 26218b7
commit 85f47ba
Show file tree

Hide file tree

Showing 10 changed files with 30 additions and 30 deletions.
diff --git a/admin/tool/dataprivacy/tests/metadata_registry_test.php b/admin/tool/dataprivacy/tests/metadata_registry_test.php
@@ -100,6 +100,6 @@ public function test_get_registry_metadata_provider_details() {
         $this->assertEquals(1, $corerating['compliant']);
         $this->assertNotEmpty($corerating['metadata']);
         $this->assertEquals('database_table', $corerating['metadata'][0]['type']);
-        $this->assertNotEmpty('database_table', $corerating['metadata'][0]['fields']);
+        $this->assertNotEmpty($corerating['metadata'][0]['fields']);
     }
 }
diff --git a/admin/tool/log/tests/privacy_test.php b/admin/tool/log/tests/privacy_test.php
@@ -66,7 +66,7 @@ public function test_get_contexts_for_userid() {
         $manager = get_log_manager(true);
 
         $this->setUser($u1);
-        $this->assertEmpty(provider::get_contexts_for_userid($u1->id)->get_contextids(), []);
+        $this->assertEmpty(provider::get_contexts_for_userid($u1->id)->get_contextids());
         $e = \logstore_standard\event\unittest_executed::create(['context' => $c1ctx]);
         $e->trigger();
         $this->assertEquals($c1ctx->id, provider::get_contexts_for_userid($u1->id)->get_contextids()[0]);

diff --git a/mod/assign/tests/events_test.php b/mod/assign/tests/events_test.php
@@ -629,7 +629,7 @@ public function test_submission_graded() {
         );
         $assign->testable_process_save_quick_grades($data);
         $grade = $assign->get_user_grade($student->id, false);
-        $this->assertEquals('60.0', $grade->grade);
+        $this->assertEquals(60.0, $grade->grade);
 
         $events = $sink->get_events();
         $this->assertCount(3, $events);
@@ -655,7 +655,7 @@ public function test_submission_graded() {
         $data->grade = '50.0';
         $assign->update_grade($data);
         $grade = $assign->get_user_grade($student->id, false, 0);
-        $this->assertEquals('50.0', $grade->grade);
+        $this->assertEquals(50.0, $grade->grade);
         $events = $sink->get_events();
 
         $this->assertCount(3, $events);

diff --git a/mod/assign/tests/externallib_test.php b/mod/assign/tests/externallib_test.php
@@ -1134,7 +1134,7 @@ public function test_save_grade() {
         $result = mod_assign_external::get_grades(array($instance->id));
         $result = external_api::clean_returnvalue(mod_assign_external::get_grades_returns(), $result);
 
-        $this->assertEquals($result['assignments'][0]['grades'][0]['grade'], '50.0');
+        $this->assertEquals((float)$result['assignments'][0]['grades'][0]['grade'], '50.0');
     }
 
     /**
@@ -1284,13 +1284,13 @@ public function test_save_grades_with_advanced_grading() {
                                          array('userid' => $student1->id, 'assignment' => $instance->id),
                                          '*',
                                          MUST_EXIST);
-        $this->assertEquals($student1grade->grade, '50.0');
+        $this->assertEquals((float)$student1grade->grade, '50.0');
 
         $student2grade = $DB->get_record('assign_grades',
                                          array('userid' => $student2->id, 'assignment' => $instance->id),
                                          '*',
                                          MUST_EXIST);
-        $this->assertEquals($student2grade->grade, '100.0');
+        $this->assertEquals((float)$student2grade->grade, '100.0');
     }
 
     /**

diff --git a/mod/assign/tests/locallib_test.php b/mod/assign/tests/locallib_test.php
@@ -3640,7 +3640,7 @@ public function test_process_save_quick_grades() {
         $result = $assign->testable_process_save_quick_grades($data);
         $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result);
         $grade = $assign->get_user_grade($student->id, false);
-        $this->assertEquals('60.0', $grade->grade);
+        $this->assertEquals(60.0, $grade->grade);
 
         // Attempt to grade with a past attempts grade info.
         $assign->testable_process_add_attempt($student->id);
@@ -3664,7 +3664,7 @@ public function test_process_save_quick_grades() {
         $result = $assign->testable_process_save_quick_grades($data);
         $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result);
         $grade = $assign->get_user_grade($student->id, false);
-        $this->assertEquals('40.0', $grade->grade);
+        $this->assertEquals(40.0, $grade->grade);
 
         // Catch grade update conflicts.
         // Save old data for later.
@@ -3678,13 +3678,13 @@ public function test_process_save_quick_grades() {
         $result = $assign->testable_process_save_quick_grades($data);
         $this->assertContains(get_string('quickgradingchangessaved', 'assign'), $result);
         $grade = $assign->get_user_grade($student->id, false);
-        $this->assertEquals('30.0', $grade->grade);
+        $this->assertEquals(30.0, $grade->grade);
 
         // Now update using 'old' data. Should fail.
         $result = $assign->testable_process_save_quick_grades($pastdata);
         $this->assertContains(get_string('errorrecordmodified', 'assign'), $result);
         $grade = $assign->get_user_grade($student->id, false);
-        $this->assertEquals('30.0', $grade->grade);
+        $this->assertEquals(30.0, $grade->grade);
     }
 
     /**

diff --git a/mod/assign/tests/privacy_test.php b/mod/assign/tests/privacy_test.php
@@ -315,8 +315,8 @@ public function test_export_user_data_student() {
         $this->assertEquals(1, $writer->get_data(['attempt 1', 'submission'])->attemptnumber);
         $this->assertEquals(2, $writer->get_data(['attempt 2', 'submission'])->attemptnumber);
         // Check grades.
-        $this->assertEquals($grade1, $writer->get_data(['attempt 1', 'grade'])->grade);
-        $this->assertEquals($grade2, $writer->get_data(['attempt 2', 'grade'])->grade);
+        $this->assertEquals((float)$grade1, $writer->get_data(['attempt 1', 'grade'])->grade);
+        $this->assertEquals((float)$grade2, $writer->get_data(['attempt 2', 'grade'])->grade);
         // Check feedback.
         $this->assertContains($teachercommenttext, $writer->get_data(['attempt 1', 'Feedback comments'])->commenttext);
         $this->assertContains($teachercommenttext2, $writer->get_data(['attempt 2', 'Feedback comments'])->commenttext);
@@ -425,11 +425,11 @@ public function test_export_user_data_teacher() {
 
         // Check for student grades given.
         $student1grade = $writer->get_data(['studentsubmissions', $user1->id, 'attempt 1', 'grade']);
-        $this->assertEquals($grade1, $student1grade->grade);
+        $this->assertEquals((float)$grade1, $student1grade->grade);
         $student2grade1 = $writer->get_data(['studentsubmissions', $user2->id, 'attempt 1', 'grade']);
-        $this->assertEquals($grade2, $student2grade1->grade);
+        $this->assertEquals((float)$grade2, $student2grade1->grade);
         $student2grade2 = $writer->get_data(['studentsubmissions', $user2->id, 'attempt 2', 'grade']);
-        $this->assertEquals($grade3, $student2grade2->grade);
+        $this->assertEquals((float)$grade3, $student2grade2->grade);
         // Check for feedback given to students.
         $this->assertContains($teachercommenttext, $writer->get_data(['studentsubmissions', $user1->id, 'attempt 1',
                 'Feedback comments'])->commenttext);

diff --git a/mod/quiz/tests/attempt_walkthrough_from_csv_test.php b/mod/quiz/tests/attempt_walkthrough_from_csv_test.php
@@ -338,7 +338,7 @@ protected function check_attempt_results($result, $attemptobj) {
                     $this->assertEquals((bool)$value, $attemptobj->is_finished());
                     break;
                 case 'summarks' :
-                    $this->assertEquals($value, $attemptobj->get_sum_marks(), "Sum of marks of attempt {$result['quizattempt']}.");
+                    $this->assertEquals((float)$value, $attemptobj->get_sum_marks(), "Sum of marks of attempt {$result['quizattempt']}.");
                     break;
                 case 'quizgrade' :
                     // Check quiz grades.

diff --git a/mod/workshop/tests/privacy_provider_test.php b/mod/workshop/tests/privacy_provider_test.php
@@ -189,19 +189,19 @@ public function test_get_contexts_for_userid() {
         // Student1 has data in workshop11 (author + self reviewer), workshop12 (author) and workshop21 (reviewer).
         $contextlist = \mod_workshop\privacy\provider::get_contexts_for_userid($this->student1->id);
         $this->assertInstanceOf(\core_privacy\local\request\contextlist::class, $contextlist);
-        $this->assertEquals([$context11->id, $context12->id, $context21->id], $contextlist->get_contextids(), null, 0.0, 10, true);
+        $this->assertEquals([$context11->id, $context12->id, $context21->id], $contextlist->get_contextids(), '', 0.0, 10, true);
 
         // Student2 has data in workshop11 (reviewer), workshop12 (reviewer) and workshop21 (author).
         $contextlist = \mod_workshop\privacy\provider::get_contexts_for_userid($this->student2->id);
-        $this->assertEquals([$context11->id, $context12->id, $context21->id], $contextlist->get_contextids(), null, 0.0, 10, true);
+        $this->assertEquals([$context11->id, $context12->id, $context21->id], $contextlist->get_contextids(), '', 0.0, 10, true);
 
         // Student3 has data in workshop11 (reviewer).
         $contextlist = \mod_workshop\privacy\provider::get_contexts_for_userid($this->student3->id);
-        $this->assertEquals([$context11->id], $contextlist->get_contextids(), null, 0.0, 10, true);
+        $this->assertEquals([$context11->id], $contextlist->get_contextids(), '', 0.0, 10, true);
 
         // Teacher4 has data in workshop12 (gradeoverby) and workshop21 (gradinggradeoverby).
         $contextlist = \mod_workshop\privacy\provider::get_contexts_for_userid($this->teacher4->id);
-        $this->assertEquals([$context21->id, $context12->id], $contextlist->get_contextids(), null, 0.0, 10, true);
+        $this->assertEquals([$context21->id, $context12->id], $contextlist->get_contextids(), '', 0.0, 10, true);
     }
 
     /**

diff --git a/question/type/multichoice/tests/helper.php b/question/type/multichoice/tests/helper.php
@@ -80,31 +80,31 @@ public static function get_multichoice_question_data_two_of_four() {
                 'id' => 13,
                 'answer' => 'One',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.5',
+                'fraction' => 0.5,
                 'feedback' => 'One is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             14 => (object) array(
                 'id' => 14,
                 'answer' => 'Two',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Two is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             15 => (object) array(
                 'id' => 15,
                 'answer' => 'Three',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.5',
+                'fraction' => 0.5,
                 'feedback' => 'Three is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             16 => (object) array(
                 'id' => 16,
                 'answer' => 'Four',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Four is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -261,31 +261,31 @@ public static function get_multichoice_question_data_one_of_four() {
                 'id' => 13,
                 'answer' => 'One',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '1',
+                'fraction' => 1,
                 'feedback' => 'One is the oddest.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             14 => (object) array(
                 'id' => 14,
                 'answer' => 'Two',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Two is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             15 => (object) array(
                 'id' => 15,
                 'answer' => 'Three',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0',
+                'fraction' => 0,
                 'feedback' => 'Three is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
             16 => (object) array(
                 'id' => 16,
                 'answer' => 'Four',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Four is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),

diff --git a/question/type/multichoice/tests/upgradelibnewqe_test.php b/question/type/multichoice/tests/upgradelibnewqe_test.php
@@ -266,7 +266,7 @@ public function test_multichoice_deferredfeedback_history960() {
                     'fraction' => 0.9,
                     'timecreated' => 1278604597,
                     'userid' => null,
-                    'data' => array('-comment' => 'Well done!', '-mark' => '0.9', '-maxmark' => '1'),
+                    'data' => array('-comment' => 'Well done!', '-mark' => '0.9', '-maxmark' => 1),
                 ),
             ),
         );