Skip to content

Detect Test Attribute in TestsNodeAnalyzer::isTestClassMethod #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 8, 2025

Conversation

Levivb
Copy link
Contributor

@Levivb Levivb commented May 7, 2025

The isTestClassMethod didn't take phpunit 12 #[Test] Attribute into account. Now it does. Tested in on a project and yielded thousands of updated method signatures spanning over 250 test files. So it seems to work :)

It uses the php 8.4 array_any function. But I think that's going to be downgraded automatically by rector, right?

@samsonasik
Copy link
Member

Please run

vendor/bin/rector && composer fix-cs

to fix ci,

also, please add fixture test

@Levivb
Copy link
Contributor Author

Levivb commented May 7, 2025

Sure thing. Will do that at home. There I can properly git clone the project (instead of working in github editor)

@Levivb
Copy link
Contributor Author

Levivb commented May 7, 2025

fixture test added and ran composer fix-cs

vendor/bin/rector blows up in my face:

$ vendor/bin/rector
Fatal error: Uncaught Error: Class "Symfony\Component\String\UnicodeString" not found in rector-phpunit/vendor/symfony/console/Helper/Helper.php:51
Stack trace:
#0 rector-phpunit/vendor/symfony/console/Helper/ProgressBar.php(405): Symfony\Component\Console\Helper\Helper::width('282')
#1 rector-phpunit/vendor/symfony/console/Helper/ProgressBar.php(77): Symfony\Component\Console\Helper\ProgressBar->setMaxSteps(282)
#2 rector-phpunit/vendor/symfony/console/Style/OutputStyle.php(43): Symfony\Component\Console\Helper\ProgressBar->__construct(Object(Symfony\Component\Console\Output\StreamOutput), 282)
#3 rector-phpunit/vendor/symfony/console/Style/SymfonyStyle.php(317): Symfony\Component\Console\Style\OutputStyle->createProgressBar(282)
#4 rector-phpunit/vendor/rector/rector-src/src/Console/Style/RectorStyle.php(35): Symfony\Component\Console\Style\SymfonyStyle->createProgressBar(282)
#5 rector-phpunit/vendor/symfony/console/Style/SymfonyStyle.php(293): Rector\Console\Style\RectorStyle->createProgressBar(282)
#6 rector-phpunit/vendor/rector/rector-src/src/Application/ApplicationFileProcessor.php(77): Symfony\Component\Console\Style\SymfonyStyle->progressStart(282)
#7 rector-phpunit/vendor/rector/rector-src/src/Console/Command/ProcessCommand.php(154): Rector\Application\ApplicationFileProcessor->run(Object(Rector\ValueObject\Configuration), Object(Symfony\Component\Console\Input\ArgvInput))
#8 rector-phpunit/vendor/symfony/console/Command/Command.php(326): Rector\Console\Command\ProcessCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 rector-phpunit/vendor/symfony/console/Application.php(1078): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#10 rector-phpunit/vendor/symfony/console/Application.php(324): Symfony\Component\Console\Application->doRunCommand(Object(Rector\Console\Command\ProcessCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 rector-phpunit/vendor/rector/rector-src/src/Console/ConsoleApplication.php(77): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 rector-phpunit/vendor/symfony/console/Application.php(175): Rector\Console\ConsoleApplication->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 rector-phpunit/vendor/rector/rector-src/bin/rector.php(158): Symfony\Component\Console\Application->run()
#14 rector-phpunit/vendor/rector/rector-src/bin/rector(4): require_once('/Users/me/rec...')
#15 rector-phpunit/vendor/bin/rector(119): include('/Users/me/rec...')
#16 {main}
  thrown in rector-phpunit/vendor/symfony/console/Helper/Helper.php on line 51

@samsonasik
Copy link
Member

You possibly have issue with composer update/install, you can see troubleshooting vendor patch here

https://github.com/symplify/vendor-patches/?tab=readme-ov-file#troubleshooting

for windows, you may need to add Git usr directory to path

@Levivb
Copy link
Contributor Author

Levivb commented May 8, 2025

That did the trick. thanks! Changes submitted

First time I saw vendor-patches in action, so it looked interactive to apply (pressing 'enter' to apply). But in reality, it didn't do anything :)

Is there a way to detect this problem so vendor-patches shows information on why it might hang?

@samsonasik
Copy link
Member

Could you add fixture to existing rules tests in this package that using the method instead of add new test class? Thank you

@Levivb
Copy link
Contributor Author

Levivb commented May 8, 2025

Sure, can you point me in the right direction? Because I didn't find a test for this rule at all

@samsonasik
Copy link
Member

You can search here, it seems at least 2 rules use of it https://github.com/search?q=repo%3Arectorphp%2Frector-phpunit%20isTestClassMethod&type=code

Then, locate fixture under rules-tests directory

@Levivb
Copy link
Contributor Author

Levivb commented May 8, 2025

Thanks. I was looking for TestWithAnnotationToAttributeRector. But there's no such test.

I've added a fixture to PHPUnit60/Rector/ClassMethod/AddDoesNotPerformAssertionToNonAssertingTestRector and to AnnotationsToAttributes/Rector/ClassMethod/TestWithAnnotationToAttributeRector

TestWithAnnotationToAttributeRectorTest is removed

@samsonasik
Copy link
Member

Let's give it a try, thank you @Levivb

@samsonasik samsonasik merged commit 462d1a9 into rectorphp:main May 8, 2025
6 checks passed
@Levivb Levivb deleted the patch-1 branch May 8, 2025 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants