Please review the patch applied.

CommentFileSizeAuthor
#15 no_assertEqual.png20.26 KBguilhermevp
#13 interdiff_11-13.txt814 byteskarishmaamin
#13 3052397-13.patch11.46 KBkarishmaamin
#11 3052397-11.patch11.46 KBsuresh prabhu parkala
#9 3052397-9.patch12.45 KBshubham.prakash
#7 3052397-7.patch19.42 KBshubham.prakash
#2 deprecated-3052397-2.patch18.97 KBDeeksha B

Comments

Deeksha B created an issue. See original summary.

Deeksha B’s picture

Deeksha B commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new deprecated-3052397-2.patch18.97 KB

Status: Needs review » Needs work

The last submitted patch, 2: deprecated-3052397-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Deeksha B’s picture

Deeksha B commented
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Postponed

It's not deprecated if it's still old simpletests. We need to convert the tests to phpunit first, that's more important, these methods are actually no longer scheduled for removal in Drupal 9.

idebr’s picture

idebr commented
Status: Postponed » Needs review

The conversion to PHPUnit was fixed in #3025986: Convert automated tests from Simpletest to PHPUnit, so this issue can now be un-postponed.

shubham.prakash’s picture

shubham.prakash commented
StatusFileSize
new 3052397-7.patch19.42 KB

Replace assertEqual() with assertEquals()

idebr’s picture

idebr commented
Status: Needs review » Needs work

Patch no longer applies after #3051124: Remove Drupal 8.7 and earlier deprecations, compatibility with Drupal 9 was committed.

shubham.prakash’s picture

shubham.prakash commented
Status: Needs work » Needs review
StatusFileSize
new 3052397-9.patch12.45 KB
ravi.shankar’s picture

ravi.shankar commented
Status: Needs review » Needs work

Setting to NW dew to test fails.

suresh prabhu parkala’s picture

suresh prabhu parkala commented
Status: Needs work » Needs review
StatusFileSize
new 3052397-11.patch11.46 KB

Please review!

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/tests/src/Functional/RedirectUITest.php
@@ -242,14 +242,14 @@ class RedirectUITest extends BrowserTestBase {
     $expected = 'http_response ' . implode(' ', $redirect1->getCacheTags());
-    $this->assertEqual($expected, $response->getHeader('x-drupal-cache-tags')[0], 'Redirect cache tags properly set.');
+    $this->assertEquals($response->getHeader('x-drupal-cache-tags')[0], $expected, 'Redirect cache tags properly set.');

almost perfect. This one has the expected argument first already, so it shouldn't be switched.

karishmaamin’s picture

karishmaamin commented
Status: Needs work » Needs review
StatusFileSize
new 3052397-13.patch11.46 KB
new interdiff_11-13.txt814 bytes
guilhermevp’s picture

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

Patch #13 addresses comment #12, removes all uses of deprecated function and passes all tests. Moving to RTBC.

guilhermevp’s picture

guilhermevp commented
StatusFileSize
new no_assertEqual.png20.26 KB

Adding a image evidence.

1

  • karishmaamin authored 0e9b9bb on 8.x-1.x 
    Issue #3052397 by shubham.prakash, karishmaamin, Deeksha B, Suresh...
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Fixed

Looks good and committed to 8.x-1.x. Thanks!

Status: Fixed » Closed (fixed)

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