Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Beakerboy created an issue. See original summary.

Beakerboy’s picture

Assigned: Beakerboy » Unassigned
Status: Active » Needs review
FileSize
1.19 KB

Patch for this issue

daffie’s picture

Status: Needs review » Reviewed & tested by the community

The change looks good to me.
It is the same solution as the one for #3134475: Avoid directly comparing string to blob in CommentIntegrationTest.
For me it is RTBC.

jungle’s picture

+++ b/core/modules/field_ui/tests/src/Kernel/EntityDisplayTest.php
@@ -638,14 +638,13 @@ public function testComponentDependencies() {
+    $this->assertEqual($variables, serialize($arguments));

One nitpick: assertEqual($actual, $expected) -> assertEquals($expected, $actual), the order of $actual and $expected should be swapped as well. $this->assertEquals(serialize($arguments), $variables);

Addressing and stay RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/field_ui/tests/src/Kernel/EntityDisplayTest.php
@@ -638,14 +638,13 @@ public function testComponentDependencies() {
+    $this->assertEqual($variables, serialize($arguments));

If we're making this change then we should unserialise $variables. It makes it easier to see what is different. If this breaks.

jungle’s picture

Status: Needs work » Needs review
FileSize
1.2 KB
686 bytes

Good point. @alexpott thanks for the review.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

The point of @alexpott has been addressed.
Back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 9346c2ff9b to 9.1.x and 32aaf3ebc9 to 9.0.x and 84bd359419 to 8.9.x. Thanks!

Backported to 8.9.x to keep tests aligned.

  • alexpott committed 9346c2f on 9.1.x
    Issue #3159739 by jungle, Beakerboy, daffie: Avoid directly comparing...

  • alexpott committed 32aaf3e on 9.0.x
    Issue #3159739 by jungle, Beakerboy, daffie: Avoid directly comparing...

  • alexpott committed 84bd359 on 8.9.x
    Issue #3159739 by jungle, Beakerboy, daffie: Avoid directly comparing...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.