Problem/Motivation

We're working towards replacing assertEqual with assertEquals.

Proposed resolution

In preparation for the final cleanup, change all assertEqual() calls involving NULL, TRUE and FALSE to more appropriate PHPUnit assertions.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3193600-10.patch25.8 KBayushmishra206
#5 interdiff_3193600_2-5.txt1.66 KBankithashetty
#5 3193600-5.patch25.78 KBankithashetty
#2 3193600-2.patch25.75 KBmondrake

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: mondrake » Unassigned
Status: Active » Needs review
StatusFileSize
new 3193600-2.patch25.75 KB

Patch

daffie’s picture

daffie commented
Status: Needs review » Needs work 
+++ b/core/modules/comment/tests/src/Functional/CommentStatisticsTest.php
@@ -69,7 +69,7 @@ public function testCommentNodeCommentStatistics() {
+    $this->assertSame('', $node->get('comment')->last_comment_name, 'The value of node last_comment_name is NULL.');

@@ -93,7 +93,7 @@ public function testCommentNodeCommentStatistics() {
+    $this->assertSame('', $node->get('comment')->last_comment_name, 'The value of node last_comment_name is still NULL.');

This is not correct. The test expects an empty string and the comment is saying with NULL

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +Novice

It's the comment that is wrong at this stage then, since the actual value is ''.

So the comment should read

'The value of node last_comment_name should be an empty string.'

ankithashetty’s picture

ankithashetty
Location Karnataka, India
commented
Status: Needs work » Needs review
StatusFileSize
new 3193600-5.patch25.78 KB
new interdiff_3193600_2-5.txt1.66 KB

Updated patch in #2 with changes suggested by #3 and #4. Kindly review.

Thanks!

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

All code changes now look good to me.
For me it is RTBC.

  • catch committed 2ef2387 on 9.2.x 
    Issue #3193600 by ankithashetty, mondrake, daffie: Convert assertEqual...
catch’s picture

catch
he/him
Primary language English
commented
Title: Convert assertEqual() calls involving NULL, TRUE and FALSE to more appropriate PHPUnit assertions » [backport] Convert assertEqual() calls involving NULL, TRUE and FALSE to more appropriate PHPUnit assertions
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Committed 2ef2387 and pushed to 9.2.x. Thanks!

Needs a re-roll for 9.1.x backport.

ayushmishra206’s picture

ayushmishra206 commented
Assigned: Unassigned » ayushmishra206

Will be working on this.

ayushmishra206’s picture

ayushmishra206 commented
Assigned: ayushmishra206 » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new 3193600-10.patch25.8 KB

Rerolled for 9.1.x, Please review.

daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

Reroll for 9.1 look good to me.

  • catch committed ec98293 on 9.1.x 
    Issue #3193600 by ankithashetty, mondrake, ayushmishra206, daffie:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed ec98293 and pushed to 9.1.x. Thanks!

catch’s picture

catch
he/him
Primary language English
commented
Title: [backport] Convert assertEqual() calls involving NULL, TRUE and FALSE to more appropriate PHPUnit assertions » Convert assertEqual() calls involving NULL, TRUE and FALSE to more appropriate PHPUnit assertions

Status: Fixed » Closed (fixed)

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