In many cases in our tests we have checks to see whether were using php7 compatible phpunit or php5 compatible phpunit.

    if (method_exists($this, 'expectException')) {
      $this->expectException(InvalidDataTypeException::class);
      $this->expectExceptionMessage('Object support when dumping a YAML file has been disabled.');
    }
    else {
      $this->setExpectedException(InvalidDataTypeException::class, 'Object support when dumping a YAML file has been disabled.');
    }

We can now remove these as we should always be able to use expectException.

There is one other example that looks like a phpunit polyfill, and is similar:

if (method_exists($this, 'setExpectedExceptionRegExp')) {
      $this->setExpectedExceptionRegExp(InvalidDataTypeException::class, '/^Object support when parsing a YAML file has been disabled/');
    }
    else {
      $this->expectException(InvalidDataTypeException::class);
      $this->expectExceptionMessageRegExp('/^Object support when parsing a YAML file has been disabled/');
    }
CommentFileSizeAuthor
#7 3079788-7.patch46.3 KBmondrake
#7 interdiff_4-7.txt1.21 KBmondrake
#4 remove_phpunit4_polyfills-3079788-3.patch45.09 KBspokje
#4 interdiff.txt1.77 KBspokje
#2 remove_phpunit4_polyfills-3079788-2.patch46.27 KBspokje

Comments

Mixologic created an issue. See original summary.

spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new remove_phpunit4_polyfills-3079788-2.patch46.27 KB

Due to #3059090: Deprecate \Drupal\Tests\PhpunitCompatibilityTrait::setExpectedException() there are a lot of these constructions around:

    if (method_exists($this, 'expectException')) {
       foo();
       bar();
    }
    else {
       foo();
       bar();
    }

Patch should got rid of those as well.

Let's see what TestBot thinks of this initial patch.

spokje’s picture

spokje
Primary language Dutch
commented
Assigned: Unassigned » spokje
spokje’s picture

spokje
Primary language Dutch
commented
StatusFileSize
new interdiff.txt1.77 KB
new remove_phpunit4_polyfills-3079788-3.patch45.09 KB

There is one other example that looks like a phpunit polyfill, and is similar:

if (method_exists($this, 'setExpectedExceptionRegExp')) {
$this->setExpectedExceptionRegExp(InvalidDataTypeException::class, '/^Object support when parsing a YAML file has been disabled/');
}
else {
$this->expectException(InvalidDataTypeException::class);
$this->expectExceptionMessageRegExp('/^Object support when parsing a YAML file has been disabled/');
}

I think the 2nd test failure in #2 proves that the above isn't a polyfill:

1) Drupal\Tests\Component\Serialization\YamlTest::testObjectSupportDisabledSymfony
Error: Call to undefined method Drupal\Tests\Component\Serialization\YamlTest::setExpectedExceptionRegExp()

/var/www/html/core/tests/Drupal/Tests/Component/Serialization/YamlTest.php:105

I've left the original, quoted code intact in patch #3, which also should fix the 1st test failure

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review
spokje’s picture

spokje
Primary language Dutch
commented
Assigned: spokje » Unassigned
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Remove phpunit4 polyfills » Remove phpunit4 polyfills from 8.8.x
StatusFileSize
new interdiff_4-7.txt1.21 KB
new 3079788-7.patch46.3 KB

setExpectedExceptionRegExp does not exist in PHPUnit 6, so leaving it like in #2 leads to test failure in 8.8.x. It should be removed and use expectException and <code>expectExceptionMessageRegExp instead.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Added this issue to the Support for PHPUnit 4 has been removed change record.

spokje’s picture

spokje
Primary language Dutch
commented

Thanks @mondrake, especially for the Explanation for Dummies :)
It showed me I'd tackled that the wrong way around.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs review » Reviewed & tested by the community

Ran it through Testbot once more to make sure it passes the latest tests.
Green Testbot = RTBC for me.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Nice tidy up. Thanks.

Committed 3655e08 and pushed to 8.8.x. Thanks!

  • alexpott committed 3655e08 on 8.8.x 
    Issue #3079788 by Spokje, mondrake: Remove phpunit4 polyfills from 8.8.x

Status: Fixed » Closed (fixed)

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