Replace usages of AssertLegacyTrait::assertIdenticalObject(), which is deprecated

Thanks for chipping away at these, this one looks simple enough.

Adding the "Deprecated assertions" tag to add this into the kanban board https://contribkanban.com/board/Deprecatedassertions

Confirmed that these are the only usages of the method; thanks!

+++ b/core/tests/Drupal/KernelTests/Core/KeyValueStore/DatabaseStorageExpirableTest.php
@@ -67,11 +67,11 @@ public function testCRUDWithExpiration() {
-    $this->assertIdenticalObject($stores[1]->get('foo'), $this->objects[2]);
+    $this->assertEquals($this->objects[2], $stores[1]->get('foo'));

In terms of scoping, I don't think we should be changing the order of the arguments here. Since the patch is only 12K we can pretend this was a separate commit, but for the rest of the issues let's be sure to avoid changes that are not strictly necessary, even on the same line.

Committed to 9.1.x. Let's make backport versions of this for 9.0.x and earlier to keep cherry-pick friendly.

I agree with #10. Order is important for the failure message to be useful, and if we don't fix this up as we go along now, it will be an enormous task in the future to double check each assertion to make sure the arguments are in the right order.

+1 to #10

+++ b/core/tests/Drupal/KernelTests/KernelTestBaseTest.php
@@ -318,4 +318,14 @@ public function testProfileModules() {
+  /**
+   * Tests the deprecation of AssertLegacyTrait::assertIdenticalObject.
...
+   * @group legacy
+   * @expectedDeprecation AssertLegacyTrait::assertIdenticalObject() is deprecated in drupal:8.0.0 and is removed from drupal:10.0.0. Use $this->assertEquals() instead. See https://www.drupal.org/node/3129738
+    */
+  public function testAssertIdenticalObject() {
+    $this->assertIdenticalObject((object) ['foo' => 'bar'], (object) ['foo' => 'bar']);
+  }

Adding the test to the base class, which means that it runs on every child test classes extended it, I don't think this is good.

#15, oops, yes, you are right, the class name contains Base, i thought it's a base class. 🤦

The patch for 9.x needs a reroll, because #3139218: Replace usages of AssertLegacyTrait::assertResponse(), which is deprecated landed.

@mondrake: You are right. #3139218: Replace usages of AssertLegacyTrait::assertResponse(), which is deprecated was only committed to the 9.1 branch.

Tested: All occurrences of $this-> assertIdenticalObject have been replaced by $this->assertEquals.
The method was already deprecated in 8.0.

All code changes look good to me for the 9.0.x patch in #14.
For me it is RTBC.

In 8.9.x branch found occurrences of $this-> assertIdenticalObject in file a/core/modules/user/tests/src/Kernel/TempStoreDatabaseTest.php

Correcting the patch.

I tested this on my local for D8.9 and D9.0 and there are zero(0) occurrences for $this-> assertIdenticalObject.
#22 and #14 looks good to me. Moving this to RTBC
Thanks

Yeah re: #9 and #10 it is manageable in a small patch like this, but see my scoping suggestions in #2867959: Replace usages of deprecated AssertLegacyTrait::assertIdentical and #3142351: Replace usages of AssertLegacyTrait::assertIdentical() in traits and base classes. If while we are reviewing we can split the assertions that have the "right" order for the new assertions but the wrong order for the old ones, that would make it a lot easier to review. Then there would be two tasks: Verify the existing assertions are correct when asserted, and check the word-diff.

For the larger patches where we actually swap arguments I think we are going to end up scripting a porcelain diff to check the changed argument.

For those working on individual issues locally, we could split the changes so each change is committed to local feature branches: e.g. git add -p or git merge -p

One for wrong order, one for right. And use those branch histories to create a patch.

Less relevant here since this is a backport, but just wanted to say I see the concern and I still think splitting the patches as we make them will be a good idea for the larger issues.

Anyway I fell asleep reviewing this last night and we're now in code freeze, but this is good to go in after the freeze. We'll backport it to 8.9.x but not 8.8.x.

Committed 1add829f3c and pushed to 9.0.x. Thanks!
Committed 57735992cf and pushed to 8.9.x. Thanks!