Use idiomatic AssertJ assertions

[quaff]

1. Replace assertThat(x.isEmpty()).isTrue() with assertThat(x).isEmpty() Search for : assertThat((.+).isEmpty()).isTrue() Replace with : assertThat($1).isEmpty()
   Search for : assertThat((.+).isEmpty()).isFalse() Replace with : assertThat($1).isNotEmpty()

2. Replace assertThat(x.iterator().next()) with assertThat(x).element(0) Search for : assertThat((.+).iterator().next()) Replace with : assertThat($1).element(0)

3. Replace assertThat(x.get(i)). with assertThat(x).element(i). Search for : assertThat((.+).get((\d+))).
   Replace with : assertThat($1).element($2).

[wilkinsona]

Thanks, @quaff.

Search for : assertThat((.+).isEmpty()).isTrue() Replace with : assertThat($1).isEmpty()
Search for : assertThat((.+).isEmpty()).isFalse() Replace with : assertThat($1).isNotEmpty()

I like this pair of changes. Thanks.

Search for : assertThat((.+).iterator().next()) Replace with : assertThat($1).element(0)

I'm neutral on this one.

Search for : assertThat((.+).get((\d+))). Replace with : assertThat($1).element($2).

I don't like this change. It's more verbose and I don't think it makes the code easier to read.

On balance, I think my preference would be to just keep the first type of change and revert the other two. Let's see what the rest of the team thinks.

[quaff]

It's more verbose and I don't think it makes the code easier to read.

It will report more meaningful error not just for readable consideration.

[wilkinsona]

Deliberately breaking one of the assertions, here's the error before the proposed change:

java.lang.AssertionError: 
Expecting actual:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests$CustomMeterObservationHandler@3f07b12c
to be an instance of:
  io.micrometer.core.instrument.observation.DefaultMeterObservationHandler
but was instance of:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.CustomMeterObservationHandler
	at org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.lambda$17(ObservationAutoConfigurationTests.java:252)

And here's the error afterwards:

java.lang.AssertionError: [List element at index 0] 
Expecting actual:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests$CustomMeterObservationHandler@3f07b12c
to be an instance of:
  io.micrometer.core.instrument.observation.DefaultMeterObservationHandler
but was instance of:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.CustomMeterObservationHandler
	at org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.lambda$17(ObservationAutoConfigurationTests.java:252)

For me, the addition of [List element at index 0] doesn't make the error any more meaningful.

[quaff]

Deliberately breaking one of the assertions, here's the error before the proposed change:

java.lang.AssertionError: 
Expecting actual:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests$CustomMeterObservationHandler@3f07b12c
to be an instance of:
  io.micrometer.core.instrument.observation.DefaultMeterObservationHandler
but was instance of:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.CustomMeterObservationHandler
	at org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.lambda$17(ObservationAutoConfigurationTests.java:252)

And here's the error afterwards:

java.lang.AssertionError: [List element at index 0] 
Expecting actual:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests$CustomMeterObservationHandler@3f07b12c
to be an instance of:
  io.micrometer.core.instrument.observation.DefaultMeterObservationHandler
but was instance of:
  org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.CustomMeterObservationHandler
	at org.springframework.boot.actuate.autoconfigure.observation.ObservationAutoConfigurationTests.lambda$17(ObservationAutoConfigurationTests.java:252)

For me, the addition of [List element at index 0] doesn't make the error any more meaningful.

It's indeed more, let's wait for feedbacks from others.

[scottfrederick]

I like 1 and 2. I do think that assertThat(foo).element(0) (or assertThat(foo).get(0)) is better than assertThat(foo.iterator().next()).

I don't like 3 either. .get(0) is more obvious than element(0).

[philwebb]

I like 1 but I don't think 2 or 3 give enough value given the number of files they change.

[quaff]

OK, I've updated the commit.

[mhalbritter]

Thank you!