openrewrite / rewrite-testing-frameworks

OpenRewrite recipes that perform common Java testing migration tasks.
Apache License 2.0
77 stars 73 forks source link

More best pratices for AssertJ #548

Closed velo closed 4 months ago

velo commented 4 months ago

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

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

timtebeek commented 4 months ago

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

velo commented 4 months ago

Appretiate that, I'm having terrible connectivity issues today, trying to download intellij for the past hour with no joy.

[image: --]

Marvin Froeder [image: https://]about.me/velo https://about.me/velo?promo=email_sig&utm_source=email_sig&utm_medium=email_sig&utm_campaign=external_links

On Mon, Jul 8, 2024 at 1:48 PM Tim te Beek @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/openrewrite/rewrite-testing-frameworks/pull/548#issuecomment-2214677909, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABBLDWANVUW2FT4EKRJ6R3ZLK7GBAVCNFSM6AAAAABKRF7TLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJUGY3TOOJQHE . You are receiving this because you were mentioned.Message ID: @.***>

velo commented 4 months ago

FWIW, I applied this clean to my source and got this PR from it https://github.com/OpenFeign/querydsl/pull/493/

velo commented 4 months ago

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 commented 4 months ago

Ah neat! I'd already pieced together the required steps for Maven once before here: https://github.com/langchain4j/langchain4j/pull/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 commented 4 months ago

@timtebeek I think I missed something https://github.com/OpenFeign/feign/pull/2479

timtebeek commented 4 months ago

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 commented 4 months ago

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