When testing for deprecation against Drupal 8.8.0, we currently get the following three warnings (see #3023170-38: Test for compatibility with Drupal 9) that seem like we can resolve them easily without requiring Drupal 8.8:

\Drupal\Core\Extension\ThemeHandlerInterface::install() is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use \Drupal\Core\Extension\ThemeInstallerInterface::install() instead. See https://www.drupal.org/node/3017233
Support for asserting against non-boolean values in ::assertFalse is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertEmpty(). See https://www.drupal.org/node/3082086
Support for asserting against non-boolean values in ::assertTrue is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use a different assert method, for example, ::assertNotEmpty(). See https://www.drupal.org/node/3082086

To resolve issues as soon as possible, we should try to resolve this right away, instead of waiting for us to depend on 8.8.

CommentFileSizeAuthor
#7 3099048-7--fixable_8_8_deprecations.patch22.86 KBdrunken monkey
#7 3099048-7--fixable_8_8_deprecations--with_deprecations_check.patch23.51 KBdrunken monkey
#7 3099048-7--fixable_8_8_deprecations--interdiff.txt6.21 KBdrunken monkey
#5 3099048-5.patch20.68 KBandreyjan
#5 3099048-interdiff--3-5.txt5.17 KBandreyjan
#3 3099048-3.patch23.48 KBandreyjan

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Parent issue: » #3023170: Test for compatibility with Drupal 9
andreyjan’s picture

andreyjan commented
Status: Active » Needs review
StatusFileSize
new 3099048-3.patch23.48 KB

Here's the patch for review.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Component: General code » Tests
Status: Needs review » Needs work

Thanks a lot, great job!
However, I see some cases where you incorrectly replaced assertTrue()/assertFalse() checks for values that are actually boolean. Would be great if you could fix that. (The return type should be documented for all our methods, in case you are unsure.)

Also, we should provide a second patch that combines this with #3023170-2: Test for compatibility with Drupal 9, to test whether this correctly resolves these three deprecations.

andreyjan’s picture

andreyjan commented
Status: Needs work » Needs review
StatusFileSize
new 3099048-interdiff--3-5.txt5.17 KB
new 3099048-5.patch20.68 KB

Updated patch.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/tests/src/Functional/CacheabilityTest.php
@@ -50,7 +50,7 @@ class CacheabilityTest extends SearchApiBrowserTestBase {
     $this->assertSession()->responseHeaderEquals('x-drupal-dynamic-cache', 'UNCACHEABLE');
-    $this->assertTrue(strpos($this->drupalGetHeader('cache-control'), 'no-cache'));
+    $this->assertNotEmpty(strpos($this->drupalGetHeader('cache-control'), 'no-cache'));

you can use assertContains(), that works both for whether something is in an array and if something is in a string.

Looking for things that drupal-check doesn't find, there are still a bunch of comments that say "entity manager" instead of entity type manager or so.

Another thing is to run tests directly with phpunit and enabled deprecation messages. I did only run the kernel tests quickly (quickly-ish, still took 10m for those 150 tests, impressive test coverage!). And all I saw where a handful of 8.8 messages and lots of deprecation messages about your own alter hooks.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 3099048-7--fixable_8_8_deprecations--interdiff.txt6.21 KB
new 3099048-7--fixable_8_8_deprecations--with_deprecations_check.patch23.51 KB
new 3099048-7--fixable_8_8_deprecations.patch22.86 KB

Great job, thanks a lot again! I now only found a single place where the call was wrong, plus a few places where other places where better calls were possible.
I also implemented Berdir’s suggestions from #6 – once again, thanks a lot for your input!

Finally, as mentioned in #4, I’m posting a combined patch that will also report all remaining deprecations.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Needs review » Fixed

Alright, looks very good.
Looking over the remaining deprecations, it seems all of them should be taken care of by #3023170: Test for compatibility with Drupal 9 and #3103570: Add $defaultTheme to all our functional tests.
So: committed.

Thanks a lot again, both of you!

Status: Fixed » Closed (fixed)

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