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

More best pratices for AssertJ #548

Merged
merged 6 commits into from
Jul 8, 2024
Merged

More best pratices for AssertJ #548

merged 6 commits into from
Jul 8, 2024

Conversation

velo
Copy link
Contributor

@velo velo commented Jul 8, 2024

What's changed?

1 - Created a new recipe that converts assertThat( true ).isEqualTo( true ) to assertThat( true ).isTrue(), same for false
2 - added new recipe to best practices on assertJ
3 - Added ChainedAssertions for Iterator hasNext

What's your motivation?

Noticed a lot of isEqualTo(true) on https://github.com/OpenFeign/querydsl/ and want to get it sorted

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

Hrmm, I'm a little short of intellij, can anyone help me out of formatting this, sorry

velo added 3 commits July 8, 2024 13:27
… This recipe will find instances of:

 -`assertThat(boolean).isEqualTo(true)` and replace them with `isTrue()`.
 -`assertThat(boolean).isEqualTo(false)` and replace them with `isFalse()`.

Signed-off-by: Marvin Froeder <[email protected]>
@timtebeek timtebeek self-requested a review July 8, 2024 16:40
@timtebeek timtebeek added enhancement New feature or request recipe Recipe request labels Jul 8, 2024
@timtebeek
Copy link
Contributor

Great start here @velo ! Much appreciated. I'll fix the above suggestions and the formatting and get that merged in after my next call

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@velo
Copy link
Contributor Author

velo commented Jul 8, 2024 via email

@velo
Copy link
Contributor Author

velo commented Jul 8, 2024

FWIW, I applied this clean to my source and got this PR from it
OpenFeign/querydsl#493

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Great additions, thanks a lot for adding these. Like how you immediately discovered the right patterns to use to fit in with the other cases already covered. And great to see this applied to querydsl already.

Let me know if you'd like to build recipe runs into your CI to keep any such issues out going forward!

@timtebeek timtebeek merged commit dc5e689 into openrewrite:main Jul 8, 2024
2 checks passed
@velo
Copy link
Contributor Author

velo commented Jul 8, 2024

Let me know if you'd like to build recipe runs into your CI to keep any such issues out going forward!

Ow, that would be awesome...

I'm recurrently running AssertJCleanup and upgradetojava21 on src/test/java contents.
Right now, sources are java 11 and java 17.... so I'm not running best praticles for either, but that could be a good idea too.

I'm planning on creating a recipe for softly.assertThat https://github.com/assertj/assertj-examples/blob/main/assertions-examples/src/test/java/org/assertj/examples/SoftAssertionsExamples.java#L46

That would also be a massive improvement to have

@timtebeek
Copy link
Contributor

Ah neat! I'd already pieced together the required steps for Maven once before here: langchain4j/langchain4j#673
You're welcome to adopt that already in a PR if you'd want me to look that over.
You can use preconditions if you'd like to limit the recipes to certain paths only, for now.
The nice thing about having this integrated is that it fully stops new issues from trickling in.

@velo
Copy link
Contributor Author

velo commented Jul 18, 2024

@timtebeek I think I missed something
OpenFeign/feign#2479

@timtebeek
Copy link
Contributor

Forgive my ignorance, but what exactly based on the link provided? I see a new test added with no immediate link to the work done here for as much as I can see in between conference sessions here.

@velo
Copy link
Contributor Author

velo commented Jul 20, 2024

Sorry, I had a much more elaborated message, and seems I messed up.

I tried to follow your advice on running recipes as part of build and getting code suggestions from it.

I'm obviously missing some basic step, as I have the files from langchain4j PR, but no code suggestions.

If you have any advice, I would appreciate

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request recipe Recipe request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants