Remove calls to deprecated method assertEqual from Config tests

Hi. I see we have about 14 almost identical issues. I’m not sure we need so many issues. Refer to https://www.drupal.org/core/scope for guidance.

Agree with @cilefen.

Close all but this one as duplicates and change this one so that it fixes all usages.

Reroll + remove change from AssertLegacyTrait

Adding reroll for patch in #14 + more replaces for instances of assertEqual

I'm reverting the change for simpletest-based tests cause they don't have the assertEquals function.
Also, we used to pass $group as 4th parameter to asserEqual https://api.drupal.org/api/drupal/core%21modules%21simpletest%21src%21Te...
however when using assertEquals this 4th parameter has now a different purpose
https://api.drupal.org/api/drupal/vendor%21phpunit%21phpunit%21src%21Fra...
https://phpunit.de/manual/current/en/appendixes.assertions.html#appendix...
Then, I'm removing the 4th parameter when used since we don't have an equivalent for the old $group parameter.

Looks like core/modules/node/tests/src/Traits/ContentTypeCreationTrait.php is also used in simpletest so reverting the change here too.

Patch looks great, but how we can make sure that all places covered?

Guess it needs to trigger_error or a kind of that

Like @andypost said in #21, we need to add deprecation. This can go in \Drupal\KernelTests\AssertLegacyTrait::assertEqual. We should probably have issues for all protected methods in that trait.

The deprecation policy is pretty clear about how we should do that.

This would solve the purpose.

That patch doesn't seem correct, it's also ~3MB bigger than the previous patch.

@borrison My bad, I'll upload a fresh one. Sorry for the inconvenience.

I'm looking into this issue.

I had difficulties rerolling the patch will try it again.

The only thing that should happen here is to replace assertEqual to assertEquals in config module: /core/modules/config.
The current patch and the ones before fail because of replacing them everywhere that is not the scope of this issue.

Instead of rerolling it, just get 8.7.x and do the replacement in the mentioned folder.

Thanks for the hint.
I produced a patch according to the suggestion in #33

@karimei this looks good for me, thanks you for your work, and please excuse the confusion at the sprint ;-)

EDIT:
#35 is a complete new patch for 8.7.x not based on the previous patches, so no interdiff is supplied.

Just updating the title to reflect correctly to the scope of the issue.
The filename looks questionable btw as this issue was about compatibility against deprecated calls and not a coding standard issue, so if someone reviews it further don't get it wrong :)
Also core committers, that's the last one of its parent and follows the other child issues' solutions.

Nice job @karimei and @mmbk!
It seems definitely an RTBC for me too.

+++ b/core/modules/config/tests/src/Functional/ConfigImportUITest.php
@@ -310,7 +310,7 @@ public function testImportDiff() {
+    $this->assertEquals(count($result), 1, "Diff UI is displaying colors.");

+++ b/core/modules/config/tests/src/Functional/ConfigInstallProfileOverrideTest.php
@@ -64,19 +64,19 @@ public function testInstallProfileConfigOverwrite() {
+    $this->assertEquals(count($tour->getTips()), 1, 'Optional configuration can be overridden. The language tour only has one tip');
...
+    $this->assertEquals(count($tour->getTips()), 3, 'Optional configuration that is not overridden is not affected.');

+++ b/core/modules/config/tests/src/Functional/SchemaConfigListenerWebTest.php
@@ -31,7 +31,7 @@ public function testConfigSchemaChecker() {
@@ -56,7 +56,7 @@ public function testConfigSchemaChecker() {

Shouldn't this just use assertCount now?

Also, assertEqual had no 'expected' value vs 'actual' value in its two parameters (which is why there is no consistency for this across tests currently), assertEquals does specify that the first parameter needs to be the expected value. This is not the case for a number of assertions here. The output for failed assertions becomes really confusing if you don't follow this standard.

@lendude, you are totally right, we did not see that the paramaters are swapped.
From core/tests/Drupal/KernelTests/AssertLegacyTrait.php Line 50ff

  /**
   * @see \Drupal\simpletest\TestBase::assertEqual()
   *
   * @deprecated Scheduled for removal in Drupal 9.0.0. Use self::assertEquals()
   *   instead.
   */
  protected function assertEqual($actual, $expected, $message = '') {
    $this->assertEquals($expected, $actual, $message);
  }

So this needs some more work

As for changing the tests to assertCount, I think this is noch the scope of this ticket.

As an aside, I'm not 100% convinced we should be doing this until #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase) is done, so until http://simpletest-countdown.org/ hits zero. Because we might just introduce new instances of assertEqual, or we will make the conversion of the last remaining tests even harder then they already are.

Patch rerolled

This should be N/R

Rerolled the previous patch and replaced 4 additional instances of assertEqual with assertEquals within ConfigEntityTest.php and ConfigInstallProfileOverrideTest.php. These changes are supplied in the interdiff.txt.

I'm not sure #45 was rerolled correctly, it seems to include the deprecated Drupal::entityManager() within ConfigInstallProfileOverrideTest, which was included in 8.7 but not 8.8. Therefore #46 contains this as well. This test passes on a fresh 8.8 install, where it uses the correct entityTypeManager(). I'll be looking into this some more.

Rerolled #35 to bring it up to 8.8.x. No additional occurrences of assertEqual were found after rerolling. Uses entityTypeManager() as per 8.8.

Setting back to Needs Review & Triggering bots.

I've added corrections according to comment #39:

- Changed parameters order in assertEquals()
- Used assertEqual() for suitable places.

Looks good!

1. +++ b/core/modules/config/tests/src/Functional/ConfigDependencyWebTest.php
   --- a/core/modules/config/tests/src/Functional/ConfigEntityFormOverrideTest.php
   +++ b/core/modules/config/tests/src/Functional/ConfigEntityFormOverrideTest.php
   
   +++ b/core/modules/config/tests/src/Functional/ConfigEntityFormOverrideTest.php
   +++ b/core/modules/config/tests/src/Functional/ConfigEntityFormOverrideTest.php
   @@ -36,7 +36,7 @@ public function testFormsWithOverrides() {
   
   @@ -36,7 +36,7 @@ public function testFormsWithOverrides() {
        $this->writeSettings($settings);
    
        // Test that the overridden label is loaded with the entity.
   -    $this->assertEqual($config_test_storage->load('dotted.default')->label(), $overridden_label);
   +    $this->assertEquals($config_test_storage->load('dotted.default')->label(), $overridden_label);
    
        // Test that the original label on the listing page is intact.
        $this->drupalGet('admin/structure/config_test');
   @@ -64,7 +64,7 @@ public function testFormsWithOverrides() {
   
   @@ -64,7 +64,7 @@ public function testFormsWithOverrides() {
        $this->assertIdentical($elements[0]->getValue(), $edited_label);
    
        // Test that the overridden label is still loaded with the entity.
   -    $this->assertEqual($config_test_storage->load('dotted.default')->label(), $overridden_label);
   +    $this->assertEquals($config_test_storage->load('dotted.default')->label(), $overridden_label);
      }
    
   

   These two examples both look like they need the parameters swapped.

1. +++ b/core/modules/config/tests/src/Functional/ConfigExportImportUITest.php
   @@ -95,7 +95,7 @@ public function testExportImport() {
          ->save();
   -    $this->assertEqual($this->config('system.site')->get('slogan'), $this->newSlogan);
   +    $this->assertEquals($this->config('system.site')->get('slogan'), $this->newSlogan);
    
        // Create a content type.
        $this->contentType = $this->drupalCreateContentType();
   @@ -140,7 +140,7 @@ public function testExportImport() {
   
   @@ -140,7 +140,7 @@ public function testExportImport() {
        $this->config('system.site')
          ->set('slogan', $this->originalSlogan)
          ->save();
   -    $this->assertEqual($this->config('system.site')->get('slogan'), $this->originalSlogan);
   +    $this->assertEquals($this->config('system.site')->get('slogan'), $this->originalSlogan);
    
   

   Also here.

2. +++ b/core/modules/config/tests/src/Functional/ConfigExportImportUITest.php
   @@ -172,7 +172,7 @@ public function testExportImport() {
   +    $this->assertEquals($this->config('system.site')->get('slogan'), $this->newSlogan);
   

   And here.

This patch should fix the issue.

@catch, thanks for the review, you are right.

@shubham.prakash, thanks, changes look good.
I'm attaching changes between your patch and patch from #52.
In the future please provide interdiff by yourself, it makes patch review simpler.
How to create interdiff: https://www.drupal.org/documentation/git/interdiff

Changes were obvious so I'm returning status to RTBC.

Committed 0ea7cee and pushed to 8.8.x. Thanks!