Use static data providers
#48283
Use static data providers
#48283
Conversation
src/Symfony/Component/Security/Core/Tests/Authentication/AuthenticationProviderManagerTest.php
Outdated
Show resolved
Hide resolved
3c3933b to
cbb8b64
Compare
|
Did you use rector or some other tool to automate the change? |
No as I didn't found an already available rector yet. I asked @TomasVotruba via Slack, but no answer until now. |
|
Unfortunately not all of them start with <?php
function rsearch($folder, $pattern)
{
$iti = new RecursiveDirectoryIterator($folder);
foreach(new RecursiveIteratorIterator($iti) as $file){
if (str_ends_with($file, $pattern) && is_file($file)) {
yield $file;
}
}
return [];
}
$files = rsearch(__DIR__.'/src', '.php'); // not all tests end with "Test"
/** @var SplFileInfo $file */
foreach ($files as $file) {
$contents = file_get_contents($file->getRealPath());
if (preg_match_all('#\@dataProvider (\w+)#', $contents, $matches)) {
foreach ($matches[1] as $dataProvider) {
$contents = preg_replace("#public function $dataProvider\(#", "public static function $dataProvider(", $contents);
}
file_put_contents($file->getRealPath(), $contents);
}
}This produces tons of broken tests, as there are many abstract provider methods implemented by derived classes, which require manual fixing and refactoring. I tried to do that but it was too tedious. I suggest writing a migration tool or a Rector rule to handle this but I don't have enough experience with that. I'm wondering if 4.4 should be the target here, as there will be no bug fixes provided by the end of this month and PHPUnit 10 will be most likely released next year so perhaps we could target 5.4. This would also reduce the number of conflicts when merging up. |
|
I opened rectorphp/rector-phpunit#126 as a feature request for such a rector |
Another problem is, that we have data providers using
Yes, I think most of the providers are also available in |
|
you should indeed target the branch. PHPUnit 10 is scheduled for February 2023, so Symfony 4.4 will be in security-only mode before this release. |
5.4 ? |
|
yeah, I meant to say "the 5.4 branch". Not sure where the number got stucked while writing... |
Yes, and it escalates quite fast as you try to make these helper methods static 💀. We should be able to refactor most of it though, but there are some providers using mocks ( |
|
Ok before going forward, let's wait for feedback in the rector issue |
Yes |
|
The tests using mocks in data providers will indeed need to be refactored manually (and they probably have a weird impact on PHPUnit). The suggested rector should be able to migrate all simple cases. Then, for remaining ones, a manual work will be needed. |
|
Do we have a automated way to do this change? Eg php-cs-fixer? |
|
Lets close this, rector has already created a rule, but I think it is first working with PHPUnit 10, not sure |
Extracted from closed PR symfony#48283 This is work, which needs to be done manually anyway.
This PR was merged into the 5.4 branch. Discussion ---------- Use static methods inside data providers | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | n/a | License | MIT | Doc PR | n/a Extracted from closed PR #48283 This is work, which needs to be done manually anyway. Commits ------- 90c8f73 Use static methods inside data providers
|
The rector rule is considered part of the rector ruleset about migrating to PHPUnit 10. But you can use it separately. PHPUnit has always supported static data providers AFAIK. |
Follows
This is the first PR, starting against
4.4branch.I am not sure yet, if I catched all of them, I searched for
provide*()and*Provider()methods in thetestsdirectory.