Replace assertions involving calls to array_search() with assertContains()/assertNotContains()

grep -Er '>assert.*array_search' .

yields two more results,

./core/tests/Drupal/Tests/Core/Plugin/Discovery/YamlDiscoveryTest.php:      $this->assertEquals(array_search($id, $this->expectedKeys), $definition['provider']);
./core/tests/Drupal/Tests/Core/Plugin/Discovery/YamlDiscoveryDecoratorTest.php:      $this->assertEquals(array_search($id, $this->expectedKeys), $definition['provider']);

but those have their own reason.

LGTM

1. +++ b/core/modules/user/tests/src/Functional/UserRolesAssignmentTest.php
   @@ -89,10 +89,10 @@ private function userLoadAndCheckRoleAssigned($account, $rid, $is_assigned = TRU
        if ($is_assigned) {
   -      $this->assertFalse(array_search($rid, $account->getRoles()) === FALSE, 'The role is present in the user object.');
   +      $this->assertContains($rid, $account->getRoles());
        }
        else {
   -      $this->assertTrue(array_search($rid, $account->getRoles()) === FALSE, 'The role is not present in the user object.');
   +      $this->assertNotContains($rid, $account->getRoles());
        }
   

   Woah the original code is very confusing lol.

2. +++ b/core/tests/Drupal/Tests/Core/Plugin/Discovery/YamlDiscoveryDecoratorTest.php
   @@ -82,7 +82,7 @@ public function testGetDefinitions() {
   -      $this->assertEquals(array_search($id, $this->expectedKeys), $definition['provider']);
   +      $this->assertContains($id, $this->expectedKeys, $definition['provider']);
   
   +++ b/core/tests/Drupal/Tests/Core/Plugin/Discovery/YamlDiscoveryTest.php
   @@ -63,7 +63,7 @@ public function testGetDefinitions() {
   -      $this->assertEquals(array_search($id, $this->expectedKeys), $definition['provider']);
   +      $this->assertContains($id, $this->expectedKeys, $definition['provider']);
   

   These conversions don't look right. I don't think they should be changed.

I don't think this is worth doing this task on Drupal 8. There's enough other tasks to get in and now 8.9.x is beta officially we don't do tasks on 8.x - of course we can make exceptions for tests but this one does not feel worth it.

Fortunately the patch for D9 didn't have the incorrect conversion I spotted in #10.

I actually would prefer to backport all these changes to 8.9.x once the BC shim is in, for parity between 8.9 and 9.0 and easier backports.

Postponing explicitly on the backport of #3126787: [D8 only] Add forwards-compatibility shim for assertInternalType() replacements in phpunit 6&7.

Oh, assertContains() doesn't appear to be in the backport set -- probably would need another patch on top of that. Which I think is worth doing. We have to maintain 8.9.x for the next year and a half so let's make it easy.

This (and all the issues like it) would be postponed on a followup from #3126787: [D8 only] Add forwards-compatibility shim for assertInternalType() replacements in phpunit 6&7 that provides the shim.

assertContains is a native PHPUnit method since at least version 4, so no need of fc shims here. NW though for #10 for the backport.

lol, thanks @mondrake for sorting that out. :)

LGTM now

OK, those remaining few seem fine.

+++ b/core/modules/user/tests/src/Functional/UserRolesAssignmentTest.php
@@ -89,10 +89,10 @@ private function userLoadAndCheckRoleAssigned($account, $rid, $is_assigned = TRU
     if ($is_assigned) {
-      $this->assertFalse(array_search($rid, $account->getRoles()) === FALSE, 'The role is present in the user object.');
+      $this->assertContains($rid, $account->getRoles());
     }
     else {
-      $this->assertTrue(array_search($rid, $account->getRoles()) === FALSE, 'The role is not present in the user object.');
+      $this->assertNotContains($rid, $account->getRoles());

Had to read the original code like three times. This is much better.

Committed the backport to 8.9.x and 8.8.x. Thanks! Sorry for the brief confusion; I was looking at too many different issues at once.