MDL-65204 phpunit: various fixes to assertions
authorEloy Lafuente (stronk7) <stronk7@moodle.org>
Wed, 27 Mar 2019 12:05:35 +0000 (13:05 +0100)
committerJun Pataleta <jun@moodle.com>
Wed, 3 Apr 2019 02:39:19 +0000 (10:39 +0800)
 Namely:
   - 3rd param of assertEquals() cannot be null.
   - Some incorrect uses of assertNotEmpty().
   - Comparing 2 strings now uses strict (===) evaluation.
Link: https://github.com/sebastianbergmann/phpunit/issues/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.

admin/tool/dataprivacy/tests/metadata_registry_test.php
admin/tool/log/tests/privacy_test.php
mod/assign/tests/events_test.php
mod/assign/tests/externallib_test.php
mod/assign/tests/locallib_test.php
mod/assign/tests/privacy_test.php
mod/quiz/tests/attempt_walkthrough_from_csv_test.php
mod/workshop/tests/privacy_provider_test.php
question/type/multichoice/tests/helper.php
question/type/multichoice/tests/upgradelibnewqe_test.php

index 2fc88a4..143d6b8 100644 (file)
@@ -100,6 +100,6 @@ class tool_dataprivacy_metadata_registry_testcase extends advanced_testcase {
         $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']);
     }
 }
index 9e5be71..ae85b86 100644 (file)
@@ -66,7 +66,7 @@ class tool_log_privacy_testcase extends provider_testcase {
         $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]);
index fe50250..9b6eb99 100644 (file)
@@ -629,7 +629,7 @@ class assign_events_testcase extends advanced_testcase {
         );
         $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 @@ class assign_events_testcase extends advanced_testcase {
         $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);
index e0b3e63..30ebeb5 100644 (file)
@@ -1134,7 +1134,7 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
         $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 @@ class mod_assign_external_testcase extends externallib_advanced_testcase {
                                          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');
     }
 
     /**
index 602d0d9..b9b7dc1 100644 (file)
@@ -3640,7 +3640,7 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $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 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $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 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
         $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);
     }
 
     /**
index 185d5aa..fe2f9aa 100644 (file)
@@ -315,8 +315,8 @@ class mod_assign_privacy_testcase extends provider_testcase {
         $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 @@ class mod_assign_privacy_testcase extends provider_testcase {
 
         // 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);
index 3556793..029dbd2 100644 (file)
@@ -338,7 +338,7 @@ class mod_quiz_attempt_walkthrough_from_csv_testcase extends advanced_testcase {
                     $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.
index e0c6d81..a59388d 100644 (file)
@@ -189,19 +189,19 @@ class mod_workshop_privacy_provider_testcase extends advanced_testcase {
         // 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);
     }
 
     /**
index 2729754..b665052 100644 (file)
@@ -80,7 +80,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 13,
                 'answer' => 'One',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.5',
+                'fraction' => 0.5,
                 'feedback' => 'One is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -88,7 +88,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 14,
                 'answer' => 'Two',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Two is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -96,7 +96,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 15,
                 'answer' => 'Three',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.5',
+                'fraction' => 0.5,
                 'feedback' => 'Three is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -104,7 +104,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 16,
                 'answer' => 'Four',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Four is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -261,7 +261,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 13,
                 'answer' => 'One',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '1',
+                'fraction' => 1,
                 'feedback' => 'One is the oddest.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -269,7 +269,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 14,
                 'answer' => 'Two',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Two is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -277,7 +277,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 15,
                 'answer' => 'Three',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0',
+                'fraction' => 0,
                 'feedback' => 'Three is odd.',
                 'feedbackformat' => FORMAT_HTML,
             ),
@@ -285,7 +285,7 @@ class qtype_multichoice_test_helper extends question_test_helper {
                 'id' => 16,
                 'answer' => 'Four',
                 'answerformat' => FORMAT_PLAIN,
-                'fraction' => '0.0',
+                'fraction' => 0.0,
                 'feedback' => 'Four is even.',
                 'feedbackformat' => FORMAT_HTML,
             ),
index f6859b7..cd94e1a 100644 (file)
@@ -266,7 +266,7 @@ class qtype_multichoice_attempt_upgrader_test extends question_attempt_upgrader_
                     '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),
                 ),
             ),
         );