Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Two recent backports both broke PHP 5 testing (but not runtime):
- #2313917: Core version key in module's .info.yml doesn't respect core semantic versioning
- #3007102: Migrating to Date-only field does not drop time value
Per @Mixologic:
The backport introduced some php5.5 and php5.6 regressions:
expectException doesnt exist in phpunit 4, so the backport tests fail.
https://www.drupal.org/pift-ci-job/1397533
I see a lot of this pattern in core currently:
if (method_exists($this, 'expectException')) { $this->expectException(\BadMethodCallException::class); $this->expectExceptionMessage('Method Drupal\Component\Datetime\DateTimePlus::diff expects parameter 1 to be a \DateTime or \Drupal\Component\Datetime\DateTimePlus object'); } else { $this->setExpectedException(\BadMethodCallException::class, 'Method Drupal\Component\Datetime\DateTimePlus::diff expects parameter 1 to be a \DateTime or \Drupal\Component\Datetime\DateTimePlus object'); }
Which seems to be the correct workaround to address those.
Proposed resolution
The two issues require the same fix and neither will pass tests without the other being fixed too. So, do that here.
We have two possible solutions so far on the two related issues:
- Add a try/catch like so:
+++ b/core/modules/datetime/tests/src/Unit/Plugin/migrate/field/DateFieldTest.php @@ -83,8 +83,13 @@ public function testDefineValueProcessPipelineException() { - $this->expectException(MigrateException::class); - $plugin->defineValueProcessPipeline($migration, 'field_date', ['type' => 'totoro']); + try { + $plugin->defineValueProcessPipeline($migration, 'field_date', ['type' => 'totoro']); + $this->fail("Expected MigrateException was not thrown"); + } + catch (\Exception $e) { + $this->assertInstanceOf(MigrateException::class, $e); + } }
- Switch back to
setExpectedException()
like so:
+++ b/core/tests/Drupal/Tests/Core/Extension/InfoParserUnitTest.php @@ -127,8 +126,7 @@ public function testMissingCoreCoreVersionRequirement() { - $this->expectException('\Drupal\Core\Extension\InfoParserException'); - $this->expectExceptionMessage("$exception_message-duplicate.info.txt"); + $this->setExpectedException(InfoParserException::class, "$exception_message-duplicate.info.txt");
Comments
Comment #5
xjmComment #6
mikelutz->setExpectedException is correct I believe. Both these classes extend UnitTestCase, which implement PhpUnitCompatibilityTrait, which override ->setExpectedException to call ->expectedException on phpunit 6, so that's the right way to go.
Comment #7
mikelutzComment #8
mikelutzMissed a few. I'm not quite sure yet on that intermittent segfault on 5.6, I assume it's unrelated, I wrote that test a long time ago, but I'll look into it.
Comment #9
xjmThanks @mikelutz -- I'm just going to commit this from NR since HEAD has now been broken for two weeks and I want to stop getting emails. :P
5.6 has a very long history of segfaulting on random stupid things like how an array is sorted; that segfault has been ongoing for more than a year and is one of the reasons I wanted to actually skip the test on the 5.6 nightly for 8.7. :P And also part of why I am so looking forward to December when we can say goodbye to those test runs permanently. :)
Comment #11
xjmTa-da!