Skip to content
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

remove abstract *Test source file after renaming to *TestCase #159

Merged
merged 1 commit into from
Feb 10, 2023
Merged

remove abstract *Test source file after renaming to *TestCase #159

merged 1 commit into from
Feb 10, 2023

Conversation

arderyp
Copy link
Contributor

@arderyp arderyp commented Feb 9, 2023

@arderyp
Copy link
Contributor Author

arderyp commented Feb 9, 2023

I am not sure if new test coverage is needed. I see there is a test to ensure that the new/renamed file was added, but I am not sure how these fixtures work and if an actual deletion is executed and, if so, how to test it.

@arderyp
Copy link
Contributor Author

arderyp commented Feb 9, 2023

@samsonasik

Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Looks good

@samsonasik
Copy link
Member

I verify locally and it seems working ok, @TomasVotruba I am merging it ;)

Thank you @arderyp

@samsonasik samsonasik merged commit d974dcc into rectorphp:main Feb 10, 2023
@TomasVotruba TomasVotruba mentioned this pull request Feb 11, 2023
@arderyp
Copy link
Contributor Author

arderyp commented Feb 14, 2023

very cool!

@samsonasik, in testing the rector set, I've noticed a deprecation/change that is not handled. PHPUnit 10 seems to have done away with the old hooks (example BeforeFirstTestHook) and says we should now use the new Event system. This is not properly documented yet, so I am not exactly sure what Rector should even do in these scenarios, but thought I should mention it to you.

Should I open a new issue for that? I may be able to work on it myself, but I am completely new to Rector, so it may be very slow progress.

@TomasVotruba
Copy link
Member

Thanks for looking into this.
Go for the pull-request 👍 We'll help you to improve the rule in small iterations.

@arderyp
Copy link
Contributor Author

arderyp commented Feb 16, 2023

commenting here for when I look back later.

Here is a great write-up/overview of the new event system, which will come in handy here:

https://localheinz.com/articles/2023/02/14/extending-phpunit-with-its-new-event-system/

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.

None yet

3 participants