Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
in #3353210: [PHPUnit 10] @dataProvider methods must be declared static and public we set all data provider methods to static, however the provider source in MigrateSourceTestBase is a special case, being an abstract method in a base class widely extended in contrib migrate modules. Having the abstract class declared static will prevent extending classes from having a version of code that works with both 10.2 and 10.3. While our BC promise does not apply to test classes, this particular case will be fairly disruptive, and we should avoid it if we can.
Steps to reproduce
Proposed resolution
Remove the abstract class and document that extending classes should implement the providerSource method as a static method. Current code will continue to work until the full switchover to phpUnit 10 in D11. This will allow modules to have code that works with both 10.2 and 10.3 by not declaring the method static, and code that works with both 10.3 and 10.4/11.0 by adding a static declaration when they are ready.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 3437129-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Issue fork drupal-3437129
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:
changes
Comments
Comment #2
mikelutzComment #4
mikelutzComment #5
smustgrave commentedNot sure I 100% know that error but phpstan error
Comment #6
berdirYeah, phpstan doesn't like this, so we need to add it to the baseline.
Comment #7
mikelutzThe @phpstan-ignore-next-line (on the line before the method declaration, not the @dataprovider line) worked when I ran the commit check script, but the CI doesn't like it, and I haven't gotten back to figuring out why yet.
Comment #8
mikelutzAh, because the commit check only checks the changed files, but another abstract class, MigrateSqlSourceTestBase extends MigrateSourceTestBase, and throws the same error now when all files are checked, and the names were similar enough that I didn't notice CI was complaining about a completely different file.. Okay, I added both to the baseline. Hopefully, this one works.
Comment #9
mikelutzComment #10
smustgrave commentedGood find! All green and change seems fine to me.
Comment #11
mondrakeComment #14
catchMakes sense, I opened #3437318: Add back Drupal\Tests\migrate\Kernel\MigrateSourceTestBase::providerSource() for 10.4 and 110/11.1 since we should be able to do that when we're going to drop 10.2 support.
Comment #16
berdirUnfortunately the BC layer here only works in one direction, when the method is declared as non-static.
When you try to make it static for D11 then the tests fail on 10.2 with:
PHP Fatal error: Cannot make non static method Drupal\Tests\migrate\Kernel\MigrateSourceTestBase::providerSource() static in class Drupal\Tests\paragraphs\Kernel\migrate\ParagraphsTypeSourceTest in /builds/project/paragraphs/tests/src/Kernel/migrate/ParagraphsTypeSourceTest.php on line 25
See https://git.drupalcode.org/project/paragraphs/-/jobs/1683642.
Can we backport this to 10.2.x as well?
Comment #17
catchI think it's safe to backport, it's almost entirely a docs change, removing an abstract method can't affect anything. However it needs a backport MR, 10.3.x commit doesn't cherry-pick cleanly.
Comment #20
mikelutzComment #21
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #22
mikelutzComment #23
berdirThanks!
Comment #26
catchCommitted/pushed to 10.2.x, thanks! Customized the commit message because it sounds scary removing a method but we're only removing an abstract method that's now causing breakage, any module already written against 10.2.x will already work exactly the same before/after, so there's no actual bc concern, just a forwards compatibility improvement.
Comment #27
berdirThanks for the quick turnaround. I can confirm that paragraph tests can be run against 10.2/10.3/11.0 (with varying amount of fails on all branches, but they *run*)