Problem/Motivation

PHPUnit8 introduced several warnings about things that are going away in PHPUnit 9. At the moment DrupalCI ignores these warnings and the tests pass. However if you run tests locally you see the warnings and think you test has failed.

For example running ./vendor/bin/phpunit -v -c ./core core/tests/Drupal/Tests/Core/Extension/ExtensionSerializationTest.php you'll see:

PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.3.13
Configuration: /Users/alex/dev/sites/drupal8alt.dev/core/phpunit.xml

Testing Drupal\Tests\Core\Extension\ExtensionSerializationTest
W.                                                                  2 / 2 (100%)

Time: 245 ms, Memory: 6.00 MB

There was 1 warning:

1) Drupal\Tests\Core\Extension\ExtensionSerializationTest::testServiceAppRouteUsage
readAttribute() is deprecated and will be removed in PHPUnit 9.
getObjectAttribute() is deprecated and will be removed in PHPUnit 9.

/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestResult.php:691
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/Framework/TestSuite.php:597
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:621
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/Command.php:200
/Users/alex/dev/sites/drupal8alt.dev/vendor/phpunit/phpunit/src/TextUI/Command.php:159

WARNINGS!
Tests: 2, Assertions: 3, Warnings: 1.

This is critical because local testing and DrupalCI testing do not match.

Proposed resolution

Remaining tasks

User interface changes

None

API changes

New test trait that our test base classes use to ignore warnings.

Data model changes

None

Release notes snippet

None

CommentFileSizeAuthor
#12 3116856-12.patch27.73 KBalexpott
#11 3116856-11.patch27.52 KBalexpott
#9 3116856-9.patch27.54 KBalexpott
#9 3-9-interdiff.txt568 bytesalexpott
#3 3116856-3.patch27.42 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Parent issue: » #3110543: [meta] Support PHPUnit 9 in Drupal 9
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 3116856-3.patch27.42 KB

Here's the patch from #3110543-33: [meta] Support PHPUnit 9 in Drupal 9.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue tags: +PHPUnit8
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Title: Workaround PHPUnit 8 warning » Workaround PHPUnit 8 warnings
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

This is critical because local testing and DrupalCI testing do not match.

Speaking of which, see: #2905007: Allow run-tests.sh to report skipped/incomplete PHPUnit tests

  1. +++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -117,6 +117,7 @@ public static function isDeprecationSkipped($message) {
+      '%The "PHPUnit\\\\Framework\\\\TestCase::addWarning\(\)" method is considered internal%',

    Depreception! 

  2. +++ b/core/tests/Drupal/Tests/Traits/PHPUnit8Warnings.php
@@ -0,0 +1,52 @@
+  public function addWarning(string $warning): void {
+    if (in_array($warning, self::$ignoredWarnings, TRUE)) {
+      return;
+    }
+    parent::addWarning($warning);
+  }

    I really wish we could turn this off easily so we can see the tech debt without having to find the deprecation trait to alter it. Does convertWarningsToExceptions or similar let us do this?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

@Mile23 I envisage that we're going to clear out this warnings very quickly. So we can support PHPUnit 9 so I don't like contrib or custom will have to wait long. So I'm not sure that turning it off is that worthwhile. convertWarningsToExceptions doesn't work.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Patch applies and works locally for me, tests are green... 

+++ b/core/tests/Drupal/Tests/Traits/PHPUnit8Warnings.php
@@ -0,0 +1,52 @@
+ * @internal

Give this annotation a neighbor that's a @todo pointing to #3110543: [meta] Support PHPUnit 9 in Drupal 9 and it's RTBC afaic.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3-9-interdiff.txt568 bytes
new 3116856-9.patch27.54 KB

@Mile23 good idea.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Reviewed & tested by the community

LGTM.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3116856-11.patch27.52 KB

I resolved the conflict in core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3116856-12.patch27.73 KB

Rerolled again.

Updated 0 paths from the index
First, rewinding head to replay your work on top of it...
Applying: Init commit
Using index info to reconstruct a base tree...
M	core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
Falling back to patching base and 3-way merge...
Auto-merging core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php

  • catch committed 00d95c7 on 9.0.x 
    Issue #3116856 by alexpott, Mile23: Workaround PHPUnit 8 warnings
catch’s picture

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

Committed 00d95c7 and pushed to 9.0.x. Thanks!

Status: Fixed » Closed (fixed)

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