Skip to content

Commit

Permalink
MDL-71036 phpunit: assertContains() now performs strict comparison
Browse files Browse the repository at this point in the history
The methods assertContains() and assertNotContains() now perform
strict (type and value) comparison, pretty much like assertSame()
does.

A couple of new assertContainsEquals() and assertNotContainsEquals()
methods have been created to provide old (non-strict) behavior, pretty
much like assertEquals() do.

Apart from replacing the calls needing a relaxed comparison to those
new methods, there are also a couple of alternative, about how to
fix this, depending of every case:

- If the test is making any array_values() conversion, then it's better
  to remove that conversion and use assertArrayHasKey(), that is not
  strict.
- Sometimes if may be also possible to, simply, cast the expectation
  to the exact type coming in the array. I've not applied this technique
  to any of the cases in core.

Link: sebastianbergmann/phpunit#3426
  • Loading branch information
stronk7 committed Mar 11, 2021
1 parent 4a1df02 commit 8a14a7b
Show file tree
Hide file tree
Showing 27 changed files with 139 additions and 143 deletions.
6 changes: 3 additions & 3 deletions admin/tool/dataprivacy/tests/api_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,9 @@ public function test_get_assigned_privacy_officer_roles() {
// There should only be one PO role.
$this->assertCount(1, $roleids);
// Confirm it contains the manager role.
$this->assertContains($managerroleid, $roleids);
$this->assertContainsEquals($managerroleid, $roleids);
// And it does not contain the editing teacher role.
$this->assertNotContains($editingteacherroleid, $roleids);
$this->assertNotContainsEquals($editingteacherroleid, $roleids);
}

/**
Expand Down Expand Up @@ -838,7 +838,7 @@ public function test_get_data_requests($usertype, $fetchall, $statuses) {
$this->assertCount($filteredcount, $filteredrequests);
// Confirm the filtered requests match the status filter(s).
foreach ($filteredrequests as $request) {
$this->assertContains($request->get('status'), $statuses);
$this->assertContainsEquals($request->get('status'), $statuses);
}

if ($numstatus > 1) {
Expand Down
16 changes: 8 additions & 8 deletions admin/tool/dataprivacy/tests/expired_contexts_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -950,8 +950,8 @@ public function test_process_course_context_with_override_unexpired_role() {
$forumlist = $userlist->get_userlist_for_component('mod_forum');
$userids = $forumlist->get_userids();
$this->assertCount(1, $userids);
$this->assertContains($student->id, $userids);
$this->assertNotContains($teacher->id, $userids);
$this->assertContainsEquals($student->id, $userids);
$this->assertNotContainsEquals($teacher->id, $userids);
return true;
}));

Expand Down Expand Up @@ -1038,8 +1038,8 @@ public function test_process_course_context_with_override_expired_role() {
$forumlist = $userlist->get_userlist_for_component('mod_forum');
$userids = $forumlist->get_userids();
$this->assertCount(1, $userids);
$this->assertContains($student->id, $userids);
$this->assertNotContains($teacher->id, $userids);
$this->assertContainsEquals($student->id, $userids);
$this->assertNotContainsEquals($teacher->id, $userids);
return true;
}));

Expand Down Expand Up @@ -1127,8 +1127,8 @@ public function test_process_course_context_with_user_in_both_lists() {
$forumlist = $userlist->get_userlist_for_component('mod_forum');
$userids = $forumlist->get_userids();
$this->assertCount(1, $userids);
$this->assertContains($student->id, $userids);
$this->assertNotContains($teacher->id, $userids);
$this->assertContainsEquals($student->id, $userids);
$this->assertNotContainsEquals($teacher->id, $userids);
return true;
}));

Expand Down Expand Up @@ -1223,8 +1223,8 @@ public function test_process_course_context_with_user_in_both_lists_expired() {
$forumlist = $userlist->get_userlist_for_component('mod_forum');
$userids = $forumlist->get_userids();
$this->assertCount(2, $userids);
$this->assertContains($student->id, $userids);
$this->assertContains($teacher->id, $userids);
$this->assertContainsEquals($student->id, $userids);
$this->assertContainsEquals($teacher->id, $userids);
return true;
}));

Expand Down
12 changes: 6 additions & 6 deletions analytics/tests/prediction_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,8 @@ public function test_ml_training_and_prediction($timesplittingid, $predictedrang
$this->assertEquals($predictedrangeindex, $predictedrange->rangeindex);
$sampleids = json_decode($predictedrange->sampleids, true);
$this->assertCount(2, $sampleids);
$this->assertContains($course1->id, $sampleids);
$this->assertContains($course2->id, $sampleids);
$this->assertContainsEquals($course1->id, $sampleids);
$this->assertContainsEquals($course2->id, $sampleids);
}
$this->assertEquals(1, $DB->count_records('analytics_used_files',
array('modelid' => $model->get_id(), 'action' => 'predicted')));
Expand Down Expand Up @@ -302,10 +302,10 @@ public function test_ml_training_and_prediction($timesplittingid, $predictedrang
$this->assertEquals($predictedrangeindex, $predictedrange->rangeindex);
$sampleids = json_decode($predictedrange->sampleids, true);
$this->assertCount(4, $sampleids);
$this->assertContains($course1->id, $sampleids);
$this->assertContains($course2->id, $sampleids);
$this->assertContains($course3->id, $sampleids);
$this->assertContains($course4->id, $sampleids);
$this->assertContainsEquals($course1->id, $sampleids);
$this->assertContainsEquals($course2->id, $sampleids);
$this->assertContainsEquals($course3->id, $sampleids);
$this->assertContainsEquals($course4->id, $sampleids);
}
$this->assertEquals(2, $DB->count_records('analytics_used_files',
array('modelid' => $model->get_id(), 'action' => 'predicted')));
Expand Down
2 changes: 1 addition & 1 deletion blog/tests/privacy_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ public function test_delete_data_for_user_empty_context() {

// Generate list of contexts for user.
$contexts = provider::get_contexts_for_userid($user->id);
$this->assertContains($context->id, $contexts->get_contextids());
$this->assertContainsEquals($context->id, $contexts->get_contextids());

// Now delete the blog entry.
$entry->delete();
Expand Down
20 changes: 10 additions & 10 deletions calendar/tests/rrule_manager_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -2437,7 +2437,7 @@ public function test_first_saturday_following_first_sunday_forever() {
$this->assertEquals($expecteddate->format('Y-m-d H:i:s'), date('Y-m-d H:i:s', $record->timestart));

// Assert that the record is either the 7th, 8th, 9th, ... 13th day of the month.
$this->assertContains(date('j', $record->timestart), $bymonthdays);
$this->assertContainsEquals(date('j', $record->timestart), $bymonthdays);
}
}

Expand Down Expand Up @@ -2494,7 +2494,7 @@ public function test_every_us_presidential_election_forever() {
$this->assertEquals($expecteddate->format('Y-m-d H:i:s'), date('Y-m-d H:i:s', $record->timestart));

// Assert that the record is either the 2nd, 3rd, 4th ... 8th day of the month.
$this->assertContains(date('j', $record->timestart), $bymonthdays);
$this->assertContainsEquals(date('j', $record->timestart), $bymonthdays);
}
}

Expand Down Expand Up @@ -2525,7 +2525,7 @@ public function test_monthly_bysetpos_3_count() {
(new DateTime('1997-11-06 09:00:00 EST'))->getTimestamp()
];
foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2603,7 +2603,7 @@ public function test_every_3hours_9am_to_5pm() {
(new DateTime('1997-09-02 15:00:00 EDT'))->getTimestamp(),
];
foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2634,7 +2634,7 @@ public function test_every_15minutes_6_count() {
(new DateTime('1997-09-02 10:15:00 EDT'))->getTimestamp(),
];
foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2663,7 +2663,7 @@ public function test_every_90minutes_4_count() {
(new DateTime('1997-09-02 13:30:00 EDT'))->getTimestamp(),
];
foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2707,7 +2707,7 @@ public function test_every_20minutes_daily_byhour_byminute_50_count() {
$this->assertCount($count, $records);

foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2751,7 +2751,7 @@ public function test_every_20minutes_minutely_byhour_50_count() {
$this->assertCount($count, $records);

foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2782,7 +2782,7 @@ public function test_weekly_byday_with_wkst_mo() {
(new DateTime('1997-08-24 09:00:00 EDT'))->getTimestamp(),
];
foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down Expand Up @@ -2815,7 +2815,7 @@ public function test_weekly_byday_with_wkst_su() {
];

foreach ($records as $record) {
$this->assertContains($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
$this->assertContainsEquals($record->timestart, $expecteddates, date('Y-m-d H:i:s', $record->timestart) . ' is not found.');
}
}

Expand Down
4 changes: 2 additions & 2 deletions cohort/tests/privacy_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ public function test_get_contexts_for_userid() {
// User is member of 2 cohorts.
$contextlist = provider::get_contexts_for_userid($user->id);
$this->assertCount(2, (array) $contextlist->get_contextids());
$this->assertContains($coursecategoryctx->id, $contextlist->get_contextids());
$this->assertContains($systemctx->id, $contextlist->get_contextids());
$this->assertContainsEquals($coursecategoryctx->id, $contextlist->get_contextids());
$this->assertContainsEquals($systemctx->id, $contextlist->get_contextids());
}

/**
Expand Down
2 changes: 1 addition & 1 deletion competency/tests/api_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -4395,7 +4395,7 @@ public function test_template_statistics() {
$leastarray = array($comp4->get('id'), $comp6->get('id'));
foreach ($result as $one) {
$this->assertInstanceOf('\core_competency\competency', $one);
$this->assertContains($one->get('id'), $leastarray);
$this->assertContainsEquals($one->get('id'), $leastarray);
}
}

Expand Down
12 changes: 6 additions & 6 deletions contentbank/tests/privacy_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,21 @@ public function test_get_contexts_for_userid() {
$contextlistids = $contextlist->get_contextids();
$this->assertCount(3, $contextlistids);
// Check the list against the expected list of contexts.
$this->assertContains($scenario->systemcontext->id, $contextlistids);
$this->assertContains($scenario->coursecategorycontext->id,
$this->assertContainsEquals($scenario->systemcontext->id, $contextlistids);
$this->assertContainsEquals($scenario->coursecategorycontext->id,
$contextlistids);
$this->assertContains($scenario->coursecontext->id, $contextlistids);
$this->assertContainsEquals($scenario->coursecontext->id, $contextlistids);

// Testing againts Teacher who has content in the one context.
$contextlist = provider::get_contexts_for_userid($scenario->teacher->id);
// There are only one context in the list.
$contextlistids = $contextlist->get_contextids();
$this->assertCount(1, $contextlistids);
// Check the againts Course Context.
$this->assertContains($scenario->coursecontext->id, $contextlistids);
$this->assertContainsEquals($scenario->coursecontext->id, $contextlistids);
// And there is not a System and Course Category Context.
$this->assertNotContains($scenario->systemcontext->id, $contextlistids);
$this->assertNotContains($scenario->coursecategorycontext->id, $contextlistids);
$this->assertNotContainsEquals($scenario->systemcontext->id, $contextlistids);
$this->assertNotContainsEquals($scenario->coursecategorycontext->id, $contextlistids);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions course/tests/courselib_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -1517,9 +1517,9 @@ public function test_moveto_module_between_hidden_sections() {
$modinfo = get_fast_modinfo($course);

// Verify that forum and page have been moved to the hidden section and quiz has not.
$this->assertContains($forum->cmid, $modinfo->sections[3]);
$this->assertContains($page->cmid, $modinfo->sections[3]);
$this->assertNotContains($quiz->cmid, $modinfo->sections[3]);
$this->assertContainsEquals($forum->cmid, $modinfo->sections[3]);
$this->assertContainsEquals($page->cmid, $modinfo->sections[3]);
$this->assertNotContainsEquals($quiz->cmid, $modinfo->sections[3]);

// Verify that forum has been made invisible.
$forumcm = $modinfo->cms[$forum->cmid];
Expand Down
10 changes: 5 additions & 5 deletions enrol/flatfile/tests/privacy_provider_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@ public function test_get_contexts_for_user() {
$contextlist = provider::get_contexts_for_userid($this->user1->id);
$this->assertEquals(2, $contextlist->count());
$contextids = $contextlist->get_contextids();
$this->assertContains($this->coursecontext1->id, $contextids);
$this->assertContains($this->coursecontext3->id, $contextids);
$this->assertContainsEquals($this->coursecontext1->id, $contextids);
$this->assertContainsEquals($this->coursecontext3->id, $contextids);

// And 1 for user2 on course2.
$contextlist = provider::get_contexts_for_userid($this->user2->id);
$this->assertEquals(1, $contextlist->count());
$contextids = $contextlist->get_contextids();
$this->assertContains($this->coursecontext2->id, $contextids);
$this->assertContainsEquals($this->coursecontext2->id, $contextids);
}

/**
Expand Down Expand Up @@ -189,8 +189,8 @@ public function test_delete_data_for_user() {
$contextlist = provider::get_contexts_for_userid($this->user1->id);
$this->assertEquals(2, $contextlist->count());
$contextids = $contextlist->get_contextids();
$this->assertContains($this->coursecontext1->id, $contextids);
$this->assertContains($this->coursecontext3->id, $contextids);
$this->assertContainsEquals($this->coursecontext1->id, $contextids);
$this->assertContainsEquals($this->coursecontext3->id, $contextids);

$approvedcontextlist = new approved_contextlist(
$this->user1,
Expand Down
4 changes: 2 additions & 2 deletions enrol/paypal/tests/privacy_provider_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ public function test_get_contexts_for_userid() {
$this->assertCount(2, $contextlist);

$contextids = $contextlist->get_contextids();
$this->assertContains($coursecontext1->id, $contextids);
$this->assertContains($coursecontext2->id, $contextids);
$this->assertContainsEquals($coursecontext1->id, $contextids);
$this->assertContainsEquals($coursecontext2->id, $contextids);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions files/tests/privacy_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function test_get_contexts_for_userid() {

$contextlist = provider::get_contexts_for_userid($user->id);
$this->assertCount(1, (array) $contextlist->get_contextids());
$this->assertContains($userctx->id, $contextlist->get_contextids());
$this->assertContainsEquals($userctx->id, $contextlist->get_contextids());
}

/**
Expand Down Expand Up @@ -223,4 +223,4 @@ public function test_delete_data_for_users() {
// The user data in userctx2 should not be deleted.
$this->assertCount(1, $userlist3);
}
}
}
8 changes: 4 additions & 4 deletions grade/grading/tests/privacy_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ public function test_get_contexts_for_userid() {
// User1 has created grading definitions for instance1 and instance2.
$contextlist = provider::get_contexts_for_userid($this->user1->id);
$this->assertCount(2, $contextlist);
$this->assertContains($this->instancecontext1->id, $contextlist->get_contextids());
$this->assertContains($this->instancecontext2->id, $contextlist->get_contextids());
$this->assertNotContains($this->instancecontext0->id, $contextlist->get_contextids());
$this->assertContainsEquals($this->instancecontext1->id, $contextlist->get_contextids());
$this->assertContainsEquals($this->instancecontext2->id, $contextlist->get_contextids());
$this->assertNotContainsEquals($this->instancecontext0->id, $contextlist->get_contextids());

// User2 has only modified grading definitions for instance2.
$contextlist = provider::get_contexts_for_userid($this->user2->id);
$this->assertCount(1, $contextlist);
$this->assertContains($this->instancecontext2->id, $contextlist->get_contextids());
$this->assertContainsEquals($this->instancecontext2->id, $contextlist->get_contextids());

// User0 hasn't created or modified any grading definition.
$contextlist = provider::get_contexts_for_userid($this->user0->id);
Expand Down
4 changes: 2 additions & 2 deletions group/tests/privacy_provider_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -719,8 +719,8 @@ public function test_get_contexts_for_userid() {

$this->assertCount(3, $contextlist);
// One of the user context is the one related to self-conversation. Let's test group contexts.
$this->assertContains($coursecontext1->id, $contextids);
$this->assertContains($coursecontext2->id, $contextids);
$this->assertContainsEquals($coursecontext1->id, $contextids);
$this->assertContainsEquals($coursecontext2->id, $contextids);
}

/**
Expand Down
Loading

0 comments on commit 8a14a7b

Please sign in to comment.