hapi-fhir
hapi-fhir copied to clipboard
Replace hamcrest with assertj
This merge request replaces practically all usages of hamcrest with assertj.
One thing I wanted to include in this MR but was unable to was to run checkstyle on all our tests and add a checkstyle to enforce no hamcrest imports.
I also tried to remove the hamcrest jar entirely, but sadly it is currently required by awaitility. I opened a ticket a ticket with awaitility asking them if they had considered switching from hamcrest to assertj.
Some tricky cases noted here:
assertThat(composition.getText().getDivAsString(), stringContainsInOrder(
"Vax 2015", "Vax 2010", "Vax 2005"
));
becomes assertThat(composition.getText().getDivAsString()).containsSubsequence("Vax 2015", "Vax 2010", "Vax 2005");
assertThat(x, either(equalTo("a"), or(equalTo("b")));
becomes assertThat(x).isIn("a", "b");
assertThat(messageString, not(stringContainsInOrder(
"extension",
"meta"
)));
becomes assertThat(messageString).doesNotContainPattern("(?s)extension.*meta");
assertThat(myCaptureQueriesListener.getSelectQueries().get(0).getSql(true, false),
either(containsString("rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='B' and rt1_0.PARTITION_ID is null or rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='A' and rt1_0.PARTITION_ID is null"))
.or(containsString("rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='A' and rt1_0.PARTITION_ID is null or rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='B' and rt1_0.PARTITION_ID is null")));
becomes assertThat(sql).satisfiesAnyOf( s -> assertThat(s).contains("rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='B' and rt1_0.PARTITION_ID is null or rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='A' and rt1_0.PARTITION_ID is null"), s -> assertThat(s).contains("rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='A' and rt1_0.PARTITION_ID is null or rt1_0.RES_TYPE='Patient' and rt1_0.FHIR_ID='B' and rt1_0.PARTITION_ID is null") );
Formatting check succeeded!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 83.42%. Comparing base (
497b9f2
) to head (5a2359a
). Report is 80 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #5911 +/- ##
============================================
+ Coverage 83.39% 83.42% +0.03%
- Complexity 26927 27117 +190
============================================
Files 1681 1694 +13
Lines 103965 104727 +762
Branches 13189 13275 +86
============================================
+ Hits 86702 87370 +668
- Misses 11613 11658 +45
- Partials 5650 5699 +49
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@fil512 I did a review of ~50 randomly chosen test files, including all contents of hapi-fhir-test-util.
Some of them, particularly those that suggest switching to assertThatThrownBy()
, isEqualTo()
or contains()
, are probably nitpicky, and you can feel free to mark them as resolved.
There are a few regarding test asserts that are always true, and some that appear to test nothing except assertj itself that I'm more interested in.
This looks like it was a long haul, @fil512 @tadgh, your patience with this is laudable!
I resolved the outstanding assertThatThrownBy() requested changes after your comments on the first few. If you find any left, they're clearly fine to resolve.