Improve entire core or module with IntelliJ structural replace

Hi :sun_with_face:

while working on the migration of JUnit 4 to 5 tests in openmrs-core I noticed a few assertions that can be improved.

I found occurrences of assertEquals(true, expression) which are best written as assertTrue(expression). (We also have assertEquals(false, expression) which are best written as assertFalse(expression) )

Since I found this in 24 files with around 98 occurrences I thought how can this be automated. And here is how:

I used IntelliJs structural find & replace

setup like this

It will search for any assertEquals(true, and put whatever is after the , into a variable. I can then reuse the variable in the replace template to rewrite the code however I want :slight_smile:

Once you click ok it will show you all occurrences found with the search template. Allow you to see a preview of the changes of every single occurrence. And also select only a subset of the result to actually do the replacement. So you are in full control of what gets changed.

I hope this inspires you to try this out and tackle some refactorings in core or in your module. Investing time in such automations is very valuable, will save you time in the long run, pain in your hands and prevent mistakes we make while working on repetitive tasks :smile:

4 Likes

Thanks @teleivo for sharing. The laziness which leads to automation is awesome! :smile:

1 Like

Actually this laziness is backfiring. I am so excited about structural replacements that I keep finding new challenges :joy:

For example assertions like

assertTrue(program.getStates().size() == 1);

Are better written as

assertThat(program.getStates(), hasSize(1));

So in case the size is not equal to one I do not only get an error message saying you wanted the expression to be true but it was false. Instead I want to see the actual size of the collection in this case. Or the actual value vs my expected one.

So I did

The beauty of it is that it also highlights the match while you edit the search template :slight_smile: You can also replace old try/catch JUnit 3 style exception testing with assertThrows (see) which is very helpful for module devs migrating to JUnit 5 :ok_hand:

2 Likes

Thanks for this @teleivo!

Thanks @teleivo did you intend to mean
assertEquals(false, expression) which are best written asassertFalse(expression) ) ??

1 Like

haha, yes good catch!

1 Like

Cool :slightly_smiling_face:,

well done for the great work in reviewing and merging Junit migration Prs

cc @achilep @sharif

1 Like

thanks @teleivo , I will take this suggestion into account throughout the rest of the migration.

Lets first migrate keeping the tests as is since its quite a lot of work and we don’t want to introduce any mistakes or get stuck making nice assertions :sweat_smile: We can improve assertions later on and benefit from JUnit 5 features as well :cake:

3 Likes

thanks @teleivo :sweat_smile: :sweat_smile: :sweat_smile: for clarification .

And interesting ones. I like your approach to addressing them. :muscle:

2 Likes

Hmm it’s really scary and it is coming soon

Thanks @teleivo

Is TRUNK-5818 still pending discusions or ready for Dev ?

1 Like

this topic here is unrelated to the ticket you linked to.

Thanks @teleivo hope this is the right place.