Comments

nikolas.tatianenko created an issue. See original summary.

nikolas.tatianenko’s picture

Title: Drupal 9 Deprecated Code Report: Unit testss » Drupal 9 Deprecated Code Report: Fixes for BrowserTestBase methods
nikolas.tatianenko’s picture

StatusFileSize
new68.36 KB
nikolas.tatianenko’s picture

Status: Active » Needs review

Changes needs review

Status: Needs review » Needs work

The last submitted patch, 3: 3055616_functional_tests-3.diff, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jonathan1055’s picture

Thanks for the patch. Can I suggest that, instead of trying to fix a whole set of test files in one patch, you focus on just one type of deprecation and replacement. This will make testing and reviewing easier, and we can commit smaller patches.

Also with just one deprecation per issue, can you add a link to the Change Record and/or documentation page in the issue summary, so that we can see what is being fixed. Your patch altered 18 test files and the four tests which failed are

Drupal\Tests\scheduler\Functional\SchedulerRequiredTest
Drupal\Tests\scheduler\Functional\SchedulerPermissionsTest
Drupal\Tests\scheduler\Functional\SchedulerPastDatesTest
Drupal\Tests\scheduler\Functional\SchedulerApiTest

Good start though. Splitting up the issue will help to make progress on this.

Jonathan

jonathan1055’s picture

Another reason for splitting out into separate issues for each problem is that you avoid repeating other people's work. In your patch you have included changes for assertText() but this has already been started in #3050086: Replace deprecated function assertText() with pageTextContains()

jonathan1055’s picture

Title: Drupal 9 Deprecated Code Report: Fixes for BrowserTestBase methods » Replace deprecated assertResponse() with assertSession()->statusCodeEquals()
Assigned: nikolas.tatianenko » jonathan1055
Status: Needs work » Needs review
StatusFileSize
new8.85 KB

After checking all the changes made in patch #3 a large proportion are now covered by other issues. After removing all those, the majority of what was left were replacing assertResponse() with assertSession()->statusCodeEquals(). Hence this issue can now be specifically for that fix.

  • jonathan1055 committed 64f3527 on 8.x-1.x
    Issue #3055616 by nikolas.tatianenko, jonathan1055: Replace deprecated...
jonathan1055’s picture

Assigned: jonathan1055 » Unassigned
Status: Needs review » Fixed

Done.

jonathan1055’s picture

Issue summary: View changes
Status: Fixed » Needs work

The second parameter is not required in statusCodeEqual() and should be removed.
Can also use $assert() when #3102092: Replace deprecated function assertNoText() with pageTextNotContains() is committed.

jonathan1055’s picture

Status: Needs work » Needs review
StatusFileSize
new8.98 KB

Patch for checking.

  • jonathan1055 committed df53d27 on 8.x-1.x
    Issue #3055616 by jonathan1055: Replace deprecated assertResponse() with...
jonathan1055’s picture

Status: Needs review » Fixed

Done.

Status: Fixed » Closed (fixed)

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