-
Notifications
You must be signed in to change notification settings - Fork 85
Handle non-@Rule TemporaryFolder fields #720
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
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
That's some interesting usage patterns you're uncovering there @amishra-u ; thanks for reporting them all here in the form of pull requests. I've asked colleagues to help out with reviews as it's at times a bit much to keep up, but know it's appreciated! 😅 |
Thanks! I’m running these recipes on a repository with around 50k test classes, so they’re definitely getting battle-tested. By the time we're done, these should be rock-solid. Appreciate you looping others in for reviews—it really helps! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions could not be made:
- src/main/resources/META-INF/rewrite/examples.yml
- lines 1011-1013
- lines 2443-2503
- lines 2576-2575
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to have this odd bit of usage cleared out as well; thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions could not be made:
- src/main/resources/META-INF/rewrite/examples.yml
- lines 2443-2503
- lines 2576-2575
}).visit(a.getTree(), ctx, a.getCursor().getParentOrThrow())) | ||
.visit(mv, ctx, getCursor().getParentOrThrow()); | ||
return (J.VariableDeclarations) annotated("@org.junit.*Rule") | ||
.asVisitor(a -> JavaTemplate.builder("@TempDir") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
What's changed?
The TemporaryFolderToTempDir recipe did not correctly handle TemporaryFolder instances that were not annotated with @rule.
Issue: When the recipe made other changes, the ChangeType incorrectly migrates
new TemporaryFolder()
tonew File()
.This PR updates the recipe to properly handle TemporaryFolder instances that are not explicitly annotated with @rule,
Checklist