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).

Comment From: 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.

Comment From: 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.

Comment From: 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.

Comment From: 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.

Comment From: 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).

Comment From: philwebb

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

Comment From: quaff

OK, I've updated the commit.

Comment From: mhalbritter

Thank you!