Replace remaining assert* involving use of count() where relevant

Thanks, @mondrake! I was about to review this, but the patch did not apply on my local.

Assigning to me. I will be working on this

Sorry! @mondrake! NW for 4 coding standards messages, see https://www.drupal.org/pift-ci-job/1697536

And it looks like that it's hard to check whether there are remaining ones to fix.

#10 patch applied successfully on my local and looks good to me. RTBC1 from my side

Thanks @mondrake.

@nitesh624, The automated testing infrastructure tells us whether the patch applies, so we do not need people to review that. It is also not sufficient criteria for the issue to be marked "Reviewed and Tested by the Community". "Looks good to me" also isn't enough information for a review credit. Next time, I'd suggest specifically describing your thoughts about the patch, including exactly what you did to review it and what you think of the kinds of changes in the patch. Thanks!

So I'm "hmmm" on this change set.

1. In some places, we're changing stuff from assertEquals() to assertSame().
2. In other places, we're changing things to assertEquals().
3. And in the rest, we're actually using assertCount().

Can we clarify the "why" in each of the three cases cases?

I do see for point 1 that assertCount() is not sufficient for cases where we're comparing the count of two items. In those places it looks like an assertSameCount() method might be nice, actually. What do you think? We could split the cases where we're comparing two counts out into their own patch to explore that.

Not sure whether we really need assertSameCount() or not. Would we even need a custom constraint or could we just wrap assertSame? And if we did add it, would people discover and use it in the future, or would we just be refactoring for the sake of it and then it would never get used again? The header one is useful because there is no existing easy method for it; assertSameCount() can be emulated by writing the assertion slightly differently, as already seen here.

Also, some of these cases feel like the tests could be refactored to be simpler, but doing that here is out of scope:

1. +++ b/core/modules/book/tests/src/Functional/BookTest.php
   @@ -305,13 +305,13 @@ public function testGetTableOfContents() {
   -    $this->assertEqual(count($options), count($expected_nids));
   +    $this->assertSame(count($expected_nids), count($options));
        $diff = array_diff($expected_nids, array_keys($options));
        $this->assertTrue(empty($diff), 'Found all expected option keys');
   

   This seems like it would be better done as a single assertEquals().

2. +++ b/core/modules/book/tests/src/Functional/BookTest.php
   @@ -305,13 +305,13 @@ public function testGetTableOfContents() {
   -    $this->assertEqual(count($options), count($expected_nids));
   +    $this->assertSame(count($expected_nids), count($options));
        $diff = array_diff($expected_nids, array_keys($options));
        $this->assertTrue(empty($diff), 'Found all expected option keys after excluding Node 3');
   

   Same.

3. +++ b/core/modules/views/tests/src/Kernel/Handler/SortDateTest.php
   @@ -190,7 +190,7 @@ public function testDateOrdering() {
   -        $this->assertEqual(count($this->dataSet()), count($view->result), 'The number of returned rows match.');
   +        $this->assertSame(count($this->dataSet()), count($view->result), 'The number of returned rows match.');
            $this->assertIdenticalResultset($view, $this->expectedResultSet($granularity, $reverse), [
   

   Is the count check redundant here? It feels like IdenticalResultSet should also check the counts!

4. +++ b/core/tests/Drupal/KernelTests/Core/Menu/MenuTreeStorageTest.php
   @@ -446,7 +446,7 @@ protected function assertMenuLink($id, array $expected_properties, array $parent
        // We need both these checks since the 2nd will pass if there are extra
        // IDs loaded in $found_children.
   -    $this->assertEqual(count($children), count($found_children), "Found expected number of children for $id");
   +    $this->assertSame(count($children), count($found_children), "Found expected number of children for $id");
        $this->assertEqual(array_intersect($children, $found_children), $children, 'Child IDs match');
   

   Again, can this just be done as a single assertEquals()?

I would call it a -0.5 for assertSameCount, I might be convinced the other way but I am leaning towards "no" for now :)

All these changes look sensible now.

I've opened #3156396: Use assertSameSize() to check same size of two countable variables for assertSameCount() - IMO this would be fine to add, but also using more appropriate assertions in this patch seems good in its own right too. Don't think it hurts to do this in two steps.

Committed d1033a9 and pushed to 9.1.x. Thanks!