Use assertSameSize() to check same size of two countable variables

@catch and @mondrake Shall you please review this patch .All the usage of "assertSame" with counts replaced by the custom function
"AssertSameCount".

Thanks
Pavnish

Thanks for starting this!

IMHO it's good to have that method in a trait, but that trait should be more 'general', and imported in the base test classes - UnitTestBase, KernelTestBase, BrowserTestBase, etc.

We need test coverage for the new method itself.

An elegant solution would be to implement a PHPUnit Constraint for the check, à la #3134671: Introduce a PHPUnit Constraint to check existence of a response header , instead of falling to assertSame.

@mondrake
Thanks for your valuable feedback and suggestion.
I have ported the patch along with the your suggestion please review this patch.
Please suggest if any changes required it would be appreciated.

Thanks
Pavnish

Thanks.

1. +++ b/core/lib/Drupal/Core/Test/AssertSameTrait.php
   @@ -0,0 +1,23 @@
   +<?php
   +
   +namespace Drupal\Core\Test;
   +
   +/**
   + * Provides methods for AssertSame.
   + */
   +trait AssertSameTrait {
   

   Let's move this under Drupal/TestTools and tentatively call it AssertionTrait. This way we can add more assert methods in the trait when the need comes.

2. +++ b/core/lib/Drupal/Core/Test/AssertSameTrait.php
   @@ -0,0 +1,23 @@
   +  /**
   +   * Asserts that two variables have same number of values.
   +   *
   +   * @param array/object $expected
   +   * @param array/object $actual
   +   * @param string $message
   +   *
   +   */
   +  protected function assertSameCount($expected, $actual, string $message = '') {
   +    return $this->assertSame(count($expected), count($actual), $message);
   +  }
   +
   
   +++ b/core/tests/Drupal/KernelTests/Core/Test/AssertSameTraitTest.php
   @@ -0,0 +1,29 @@
   +  /**
   +   * Tests that the Assert same trait functions.
   +   */
   +  public function testAssertSameTrait() {
   +    $expected = ['expected'];
   +    $actual = ['actual'];
   +    // Should return Null.
   +    $assertSameStatus = $this->assertSameCount($expected,$actual);
   +    $this->assertNull($assertSameStatus, 'Expected and Actual have same count.' );
   +  }
   +
   

   We need to look at how PHPUnit assert methods are written, ussing Constraint and Assert classes. Drupal\Tests\WebAssert::responseHeaderExists could be a reference point. Also, Drupal\FunctionalTests\WebAssertTest shows how to test a custom assertion by intercepting the failure exception.

3. In addition, we should also implement the symmetrical method assertNotSameCount.

@mondrake
Thanks for the feedback and suggestions.
Point #7.1 and #7.2 addressed in this patch please review.
#7.3 :I think we should not work on assertNotSameCount method.I have tried to found the occurrence of assertNotSame with count Ex.
assertNotSame(count($foo), count($bar)) but i did not get any result.

Do we really need to invent a new assertion method? What about assertSameSize(), which already exists?

#11 interesting! This assertions is there, but totally un-documented in PHPUnit manual. It happened already once to me: https://github.com/sebastianbergmann/phpunit-documentation-english/issue...

@TR nice find, opened https://github.com/sebastianbergmann/phpunit-documentation-english/issue...

Shall we rescope this issue to use ::assertSameSize()?

Here's a patch to use assertSameSize() when comparing the count() of two arrays (or Countable or Traversable objects).

I removed the "Needs tests" tag because we're not adding new code anymore, just replacing the assertions.

I made an editing error in patch #15, resulting in a syntax error, and I also found one more assert that could be changed to use assertSameSize(). Here is a new patch and interdiff.

Please ignore #17, not my best work. That "one more assert that could be changed" wasn't in a test, so it broke everything.

Here's another patch, which is the same as #15 but with the syntax error fixed.

There's a few more that could benefit readibility by being converted.

assertSameSize() exists since phpunit 3.6.0RC1, per the commit

It's ok back-porting down to 9.1.x or even 8.x, not 100% sure if there are more to do, so far so good, RTBC+1. Thanks!

Found two more that I think can be converted by searching for assert.*count.*count, a lot of false positives to sift through though!

core/modules/layout_builder/tests/src/FunctionalJavascript/BlockFilterTest.php:    $this->assertCount($blocks_count, $visible_rows);
core/modules/migrate/tests/src/Unit/process/SubProcessTest.php:    $this->assertCount(count($process_configuration['process']), $new_value[42]);

On this

Addressed #21, thanks

There are perhaps more that we could convert, but they are going to be hard to find. This at least gives hints to others that read our tests that this method is available, so RTBC from me.