Follow up to #3594028: PHPUnit 13.2 - Replace expectExceptionMessage() with expectExceptionMessageIs*()

Problem/Motivation

PHPUnit 13.2 deprecates TestCase::expectExceptionMessage() in favor of TestCase::expectExceptionMessageIs() (stricter) or TestCase::expectExceptionMessageIsOrContains() (equivalent to TestCase::expectExceptionMessage()).

See Improved API for exception message expectations

Proposed resolution

In #3594028: PHPUnit 13.2 - Replace expectExceptionMessage() with expectExceptionMessageIs*() we replaced instances of TestCase::expectExceptionMessage() in core with a forward-compatibility implementation of TestCase::expectExceptionMessageIs().

That is OK on PHPUnit 12, but for PHPUnit 13 there are a number of cases where that is too strict and we should either make the expected message exact, of use the more permissive TestCase::expectExceptionMessageIsOrContains().

Exclude tests from the unit-component test suite and for tests extending directly from TestCase for the moment.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3611073

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes

mondrake opened merge request !16332

mondrake opened merge request !16333

mondrake closed merge request !16332

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
quietone’s picture

quietone commented
Version: 11.x-dev » main

Surely this should be on 'main', and the MR is against 'main.

mondrake’s picture

mondrake
Location 🇮🇹
commented

#7 yes, and that’s the reason I completely messed up the first MR. I cloned the issue from the parent and did not fix the version field.

dcam’s picture

dcam commented

To test this MR I upgraded PHPUnit to 13.2 on my local environment, removed the expectExceptionMessageIs() and expectExceptionMessageIs() overrides from DrupalTestCaseTrait, and with the main branch checked out I executed all of the tests changed by the MR. They failed with 84 total failures all of which were related to incorrect exception messages. After applying the MR they passed with no failures.

The only aberrant test was Drupal\Tests\Core\Test\TestDiscoveryTest. That test did not fail on the main branch. But those two edited lines did fail after applying the MR. The same thing happened on multiple test runs. But all of the tests passed on GitLab CI. I haven't been able to figure out the cause for this discrepancy. Do you mind trying to replicate my results? Here's my test output:

There were 2 failures:

1) Drupal\Tests\Core\Test\TestDiscoveryTest::testTestInfoParserMissingGroup
Failed asserting that exception message 'Missing group metadata in test Drupal\Tests\Core\Foo\MissingAttributesTest::testNoMetadata' is 'Missing group metadata in test Drupal\Tests\Core\Foo\MissingAttributesTest'.

2) Drupal\Tests\Core\Test\TestDiscoveryTest::testTestInfoParserMissingGroupWithDataProvider
Failed asserting that exception message 'Missing group metadata in test Drupal\Tests\Core\Foo\MissingAttributesWithDataProviderTest::testNoGroupMetadata#Test#1' is 'Missing group metadata in test Drupal\Tests\Core\Foo\MissingAttributesWithDataProviderTest'.

For what it's worth, all of the changes look OK to me. I didn't see anything to comment about.

mondrake’s picture

mondrake
Location 🇮🇹
commented

#9 thank you. Yeah the changes in that test seem wrong, at least for PHPUnit 12. Reverted here. Let's check again on PHPUnit 13 when the supporting MR is rebased after this one merged.

dcam’s picture

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

Great, thank you for double-checking it. Now all of the tests pass on the MR. This looks good to me.