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

Alek/remove duplicate test templates #356

Merged
merged 3 commits into from
Jun 16, 2023

Conversation

AlekSimpson
Copy link
Contributor

@AlekSimpson AlekSimpson commented Jun 15, 2023

What's changed?

Remove duplicates uses of @testtemplate implementations for a single method.

What's your motivation?

Fixes #314

Anything in particular you'd like reviewers to focus on?

I think maybe there could be more test cases but I can't think of anymore scenarios to test right now.

Anyone you would like to review specifically?

@timtebeek

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ auto-formatter on affected files
  • I've updated the documentation (if applicable)

@AlekSimpson AlekSimpson added the recipe Recipe request label Jun 15, 2023
@AlekSimpson AlekSimpson self-assigned this Jun 15, 2023
@AlekSimpson AlekSimpson marked this pull request as draft June 15, 2023 20:44
@timtebeek timtebeek marked this pull request as ready for review June 16, 2023 09:01
@timtebeek
Copy link
Contributor

Nice work! I've added a small polish commit in a0cf7ff

  • Use the full type of RepeatedTest in UsesType to limit when the recipe matches and is applied, otherwise we match too broadly, and evaluate a lot of test methods that most likely do not yet use RepeatedTest.
  • Reuse the RemoveAnnotationVisitor rather than filtering the leading annotations; we have a lot of such small building blocks, and it's easier to write and maintain recipes when we use them as much as sensible
  • Flipped the if condition to use anyMatch instead of a double noneMatch; reads clearer and should match marginally quicker
  • Added a test removeDuplicateOnly which contains a mix of methods that should and should not have changes applied, such that we guarantee we only remove annotations from the target method, not any method if the visitor matches, mostly as a safe guard if there's further changes.
  • renamed the methods in the test templates from void TestMethod to void testMethod to better match what we expect in Java.
  • Updated the copyright year and template for 2023; should only have to be changed once, but looked odd to see 2021 still.

@timtebeek timtebeek merged commit 6219e81 into main Jun 16, 2023
1 check passed
@timtebeek timtebeek deleted the alek/RemoveDuplicateTestTemplates branch June 16, 2023 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove duplicates uses of @TestTemplate implementations for a single method
2 participants