Skip to content

[PHPUnit] Create AbstractPHPUnitRector#300

Merged
TomasVotruba merged 6 commits intomasterfrom
phpunit-rectors
Jan 23, 2018
Merged

[PHPUnit] Create AbstractPHPUnitRector#300
TomasVotruba merged 6 commits intomasterfrom
phpunit-rectors

Conversation

@carusogabriel
Copy link
Copy Markdown
Contributor

@carusogabriel carusogabriel commented Jan 23, 2018

This PR solves some problems with PHPUnit Rectors:

  • Some assertions weren't being refactored if they extended a BaseTestCase class, not PHPUnit's;
  • Unify how we candidate our classes to look up for assertions: end with Test, as it is the standard in PHPUnit. In the feature, we can remove this check, looking up just for the assertions, but only when improving the speed of Rector.

After that, a bug arises calling toString in FuncCall that are actually variables: $method($argument), also fixed with this PR.

@carusogabriel carusogabriel changed the title [WIP] Create AbstractPHPUnitRector [PHPUnit] Create AbstractPHPUnitRector Jan 23, 2018
@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Jan 23, 2018

Like it overall! Just one comment to add

Have you tested that on real project? Does it pass?

{
$className = $node->getAttribute(Attribute::CLASS_NAME);

return Strings::endsWith((string) $className, 'Test');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add comment there, that this is only approximate assumption?
It might be important in some edge cases, where these Rectorswould fail, so it's easier to debug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the note that from PR would be great: https://phpunit.de/getting-started-with-phpunit.html

@carusogabriel
Copy link
Copy Markdown
Contributor Author

@TomasVotruba Composer is a proof of concept of this PR. Run with master branch, and them with this fork, and you'll see the results!

@TomasVotruba
Copy link
Copy Markdown
Member

Awesome, thanks 👍

@TomasVotruba TomasVotruba merged commit 19afd3d into master Jan 23, 2018
@carusogabriel carusogabriel deleted the phpunit-rectors branch January 23, 2018 17:47
@carusogabriel carusogabriel self-assigned this Oct 14, 2018
TomasVotruba added a commit that referenced this pull request May 26, 2019
TomasVotruba added a commit that referenced this pull request May 26, 2019
TomasVotruba added a commit that referenced this pull request May 26, 2019
🎉 Rule #300  - [CodeQuality] Add CompleteDynamicPropertiesRector
TomasVotruba added a commit that referenced this pull request Jun 26, 2021
rectorphp/rector-src@04981ba Rename ReflectionAstResolver to simple AstResolver (#300)
echo511 pushed a commit to echo511/rector that referenced this pull request Sep 4, 2021
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