CommentFileSizeAuthor
#5 interdiff-4-5.txt482 byteshctom
#5 3064818-5.patch1.92 KBhctom
#4 3064818-4.patch1.62 KBsuresh prabhu parkala
#2 3064818-2.patch1.62 KBDeeksha B

Comments

Deeksha B created an issue. See original summary.

Deeksha B’s picture

Deeksha B commented
Status: Active » Needs review
StatusFileSize
new 3064818-2.patch1.62 KB

Please review the patch.

hctom’s picture

hctom
Primary language German
commented
Status: Needs review » Needs work

Looks almost good to me: All occurrences of assertEqual are replaced with assertEquals and the tests are still working as expected. But the parameter order of the new assertEquals has changed, and so the passed parameters should reflect that.

The new order is: assertEquals($expected, $actual, $message).

This should be changed before merging... even if the deprecation is for Drupal 10 and therefore not that urgent ;)

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3064818-4.patch1.62 KB

Please review!

hctom’s picture

hctom
Primary language German
commented
StatusFileSize
new 3064818-5.patch1.92 KB
new interdiff-4-5.txt482 bytes

The actual changes for the argument order in assertEquals() calls look good now, but I identified some deprecation notices with the current test. Therefore here is another updated patch that fixes them as well.

  • mpriscella committed 612e058 on 8.x-1.x authored by hctom 
    Issue #3064818 by hctom, Deeksha B, Suresh Prabhu Parkala: Replace...
mpriscella’s picture

mpriscella commented
Status: Needs review » Fixed

Looks good! Merged into dev, will be cutting a release soon.

Status: Fixed » Closed (fixed)

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