Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply AssertJ best practices #1031

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Apply AssertJ best practices #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ...lsverifier/internal/instantiation/vintage/reflection/RecordObjectAccessorCopyingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ void copyHappyPath() {
Object original = instantiate(SimpleRecord.class);
Object copy = copyOf(original);

assertThat(copy).isNotSameAs(original);
assertThat(copy).isEqualTo(original);
assertThat(copy).isNotSameAs(original).isEqualTo(original);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can chain assertions on the same element; I think that looks best with each on a new line, but spotless disagrees.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would look better that way. It's hard to find a good formatter configuration that works well in all situations...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing // to the rescue 😄

Suggested change
assertThat(copy).isNotSameAs(original).isEqualTo(original);
assertThat(copy) //
.isNotSameAs(original)
.isEqualTo(original);

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, that's not a bad workaround, I'll keep it in mind 😄

}

@Test
Expand Down
6 changes: 3 additions & 3 deletions ...t/java/nl/jqno/equalsverifier/integration/operational/MultipleTypeEqualsVerifierTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ void succeed_whenCallingForPackageRecursivelyOnAPackageContainingFailingClasses_
void succeed_whenReportingOnSeveralCorrectClasses() {
List<EqualsVerifierReport> reports = EqualsVerifier.forClasses(A.class, B.class, C.class).report();

assertThat(reports.size()).isEqualTo(3);
assertThat(reports).hasSize(3);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now when this fails you'll get a message saying "a collection was expected with three elements, but it was a collection with x elements, containing elem1, elem2, ....". Much more helpful than expected 3 but was 5.

assertSuccessful(reports.get(0), A.class);
assertSuccessful(reports.get(1), B.class);
assertSuccessful(reports.get(2), C.class);
Expand All @@ -251,7 +251,7 @@ void succeed_whenReportingOnSeveralCorrectClasses() {
void fail_whenReportingOnOneIncorrectClass() {
List<EqualsVerifierReport> reports = EqualsVerifier.forClasses(A.class, IncorrectM.class, C.class).report();

assertThat(reports.size()).isEqualTo(3);
assertThat(reports).hasSize(3);
assertSuccessful(reports.get(0), A.class);
assertSuccessful(reports.get(2), C.class);
assertUnsuccessful(reports.get(1), IncorrectM.class, "Subclass: equals is not final.");
Expand All @@ -262,7 +262,7 @@ void fail_whenReportingOnTwoIncorrectClasses() {
List<EqualsVerifierReport> reports =
EqualsVerifier.forClasses(A.class, IncorrectM.class, C.class, IncorrectN.class).report();

assertThat(reports.size()).isEqualTo(4);
assertThat(reports).hasSize(4);
assertSuccessful(reports.get(0), A.class);
assertSuccessful(reports.get(2), C.class);
assertUnsuccessful(reports.get(1), IncorrectM.class, "Subclass: equals is not final.");
Expand Down
2 changes: 1 addition & 1 deletion ...-core/src/test/java/nl/jqno/equalsverifier/integration/operational/OriginalStateTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void allValuesReturnToOriginalState_whenEqualsVerifierIsFinishedWithException()
}
catch (AssertionError e) {
// Make sure EV fails on a check that actually mutates fields.
assertThat(e.getMessage().contains("Mutability")).isTrue();
assertThat(e.getMessage()).contains("Mutability");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion failure here would go from expected true was false to expected a string containing Mutability, but was ....

}
catch (Throwable ignored) {
fail("EqualsVerifier should have failed on FailingEqualsContainerContainer with a different exception.");
Expand Down
2 changes: 1 addition & 1 deletion ...erifier-core/src/test/java/nl/jqno/equalsverifier/integration/operational/ReportTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ void isEmptyWhenClassIsCorrect() {

assertThat(report.getType()).isEqualTo(FinalPoint.class);
assertThat(report.isSuccessful()).isTrue();
assertThat(report.getMessage()).isEqualTo("");
assertThat(report.getMessage()).isEmpty();
assertThat(report.getCause()).isNull();
}

Expand Down
2 changes: 1 addition & 1 deletion ...va/nl/jqno/equalsverifier/internal/exceptions/EqualsVerifierInternalBugExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private void assertMessagePreamble() {
}

private void assertNoCause() {
assertThat(actual.getCause()).isEqualTo(null);
assertThat(actual.getCause()).isNull();
}

private void assertCause(Throwable cause) {
Expand Down
2 changes: 1 addition & 1 deletion ...r-core/src/test/java/nl/jqno/equalsverifier/internal/exceptions/NoValueExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ class NoValueExceptionTest {
void description() {
TypeTag tag = new TypeTag(String.class);
NoValueException e = new NoValueException(tag);
assertThat(e.getDescription().contains("String")).isTrue();
assertThat(e.getDescription()).contains("String");
}
}
2 changes: 1 addition & 1 deletion ...core/src/test/java/nl/jqno/equalsverifier/internal/exceptions/RecursionExceptionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ void descriptionContainsAllTypes() {
String message = new RecursionException(stack).getDescription();

for (TypeTag tag : stack) {
assertThat(message.contains(tag.toString())).isTrue();
assertThat(message).contains(tag.toString());
}
}
}
3 changes: 1 addition & 2 deletions ...-core/src/test/java/nl/jqno/equalsverifier/internal/instantiation/SubjectCreatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ void copy() {
SomeClass original = new SomeClass(I_RED, I_RED, S_RED);
actual = sut.copy(original);

assertThat(actual).isEqualTo(expected);
assertThat(actual).isNotSameAs(expected);
assertThat(actual).isEqualTo(expected).isNotSameAs(expected);
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion .../java/nl/jqno/equalsverifier/internal/instantiation/vintage/VintageValueProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void sanityTestFactoryIncreasesStringLength() {
@Test
void provide() {
Optional<Tuple<Point>> actual = vp.provide(POINT_TAG);
assertThat(actual.get()).isEqualTo(Tuple.of(new Point(42, 42), new Point(1337, 1337), new Point(42, 42)));
assertThat(actual).contains(Tuple.of(new Point(42, 42), new Point(1337, 1337), new Point(42, 42)));
}

@Test
Expand Down
8 changes: 4 additions & 4 deletions ...rifier/internal/instantiation/vintage/reflection/InPlaceObjectAccessorScramblingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,14 @@ void scramblePrivateFinalPoint() {
void scrambleNestedGenerics() {
GenericContainerContainer foo = new GenericContainerContainer();

assertThat(foo.strings.ts.isEmpty()).isTrue();
assertThat(foo.points.ts.isEmpty()).isTrue();
assertThat(foo.strings.ts).isEmpty();
assertThat(foo.points.ts).isEmpty();

doScramble(foo);

assertThat(foo.strings.ts.isEmpty()).isFalse();
assertThat(foo.strings.ts).isNotEmpty();
assertThat(foo.strings.ts.get(0).getClass()).isEqualTo(String.class);
assertThat(foo.points.ts.isEmpty()).isFalse();
assertThat(foo.points.ts).isNotEmpty();
assertThat(foo.points.ts.get(0).getClass()).isEqualTo(Point.class);
}

Expand Down
24 changes: 12 additions & 12 deletions ...src/test/java/nl/jqno/equalsverifier/internal/reflection/ConditionalInstantiatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ void resolveReturnsNull_whenTypeDoesntExist() {
ci = new ConditionalInstantiator(THIS_TYPE_DOES_NOT_EXIST);

Class<?> actual = ci.resolve();
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -49,7 +49,7 @@ void objectIsInstantiatedCorrectly_whenValidConstructorParametersAreProvided() {
void nullIsReturned_whenInstantiateIsCalled_givenTypeDoesNotExist() {
ci = new ConditionalInstantiator(THIS_TYPE_DOES_NOT_EXIST);
Object actual = ci.instantiate(classes(String.class), objects("nope"));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -65,7 +65,7 @@ void nullIsReturned_whenInvalidConstructorParametersAreProvided_givenFalse() {
ci = new ConditionalInstantiator("java.util.GregorianCalendar", false);

Object actual = ci.instantiate(classes(int.class, int.class, int.class), objects(1999, 31, "hello"));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -81,7 +81,7 @@ void objectIsInstantiatedCorrectly_whenValidFactoryMethodAndParametersAreProvide
void nullIsReturned_whenFactoryIsCalled_givenTypeDoesNotExist() {
ci = new ConditionalInstantiator(THIS_TYPE_DOES_NOT_EXIST);
Object actual = ci.callFactory("factory", classes(String.class), objects("nope"));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -97,7 +97,7 @@ void nullIsReturned_whenInvalidMethodNameIsProvided_givenFalse() {
ci = new ConditionalInstantiator("java.lang.Integer", false);

Object actual = ci.callFactory("thisMethodDoesntExist", classes(int.class), objects(42));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -113,7 +113,7 @@ void nullIsReturned_whenInvalidFactoryMethodParametersAreProvided_givenFalse() {
ci = new ConditionalInstantiator("java.lang.Integer", false);

Object actual = ci.callFactory("valueOf", classes(int.class, int.class), objects(42));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -129,7 +129,7 @@ void objectIsInstantiatedCorrectly_whenValidExternalFactoryMethodAndParametersAr
void nullIsReturned_whenExternalFactoryIsCalled_givenTypeDoesNotExist() {
ci = new ConditionalInstantiator(THIS_TYPE_DOES_NOT_EXIST);
Object actual = ci.callFactory("java.util.Collections", "emptyList", classes(), objects());
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -145,7 +145,7 @@ void nullIsReturned_whenExternalFactoryIsCalled_givenFactoryTypeDoesNotExist_giv
ci = new ConditionalInstantiator("java.util.List", false);

Object actual = ci.callFactory("java.util.ThisTypeDoesNotExist", "emptyList", classes(), objects());
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -162,7 +162,7 @@ void nullIsReturned_whenInvalidExternalFactoryMethodNameIsProvided_givenFalse()
ci = new ConditionalInstantiator("java.util.List", false);

Object actual = ci.callFactory("java.util.Collections", "thisMethodDoesntExist", classes(), objects());
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -179,14 +179,14 @@ void nullIsReturned_whenInvalidExternalFactoryMethodParametersAreProvided_givenF
ci = new ConditionalInstantiator("java.util.List", false);

Object actual = ci.callFactory("java.util.Collections", "emptyList", classes(int.class), objects(42));
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
void nullIsReturned_whenReturnConstantIsCalled_givenTypeDoesNotExist() {
ci = new ConditionalInstantiator(THIS_TYPE_DOES_NOT_EXIST);
Object actual = ci.returnConstant("NOPE");
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}

@Test
Expand All @@ -210,6 +210,6 @@ void nullIsReturned_whenConstantDoesNotExist_givenFalse() {
ci = new ConditionalInstantiator("java.math.BigDecimal", false);

Object actual = ci.returnConstant("FORTY-TWO");
assertThat(actual).isEqualTo(null);
assertThat(actual).isNull();
}
}
10 changes: 5 additions & 5 deletions ...fier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/FieldIterableTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void onlySubClassFields() {
@Test
void noFields() {
FieldIterable iterable = FieldIterable.of(NoFields.class);
assertThat(iterable.iterator().hasNext()).isFalse();
assertThat(iterable.iterator()).isExhausted();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again; we're being more expressive about what we expect, such that you get better failure messages.

}

@Test
Expand Down Expand Up @@ -113,7 +113,7 @@ void orderingTest() {
@Test
void interfaceTest() {
FieldIterable iterable = FieldIterable.of(Interface.class);
assertThat(iterable.iterator().hasNext()).isFalse();
assertThat(iterable.iterator()).isExhausted();
}

@Test
Expand All @@ -129,13 +129,13 @@ void nextAfterLastElement() {
@Test
void objectHasNoElements() {
FieldIterable iterable = FieldIterable.of(Object.class);
assertThat(iterable.iterator().hasNext()).isFalse();
assertThat(iterable.iterator()).isExhausted();
}

@Test
void ignoreSyntheticFields() {
FieldIterable iterable = FieldIterable.of(Outer.Inner.class);
assertThat(iterable.iterator().hasNext()).isFalse();
assertThat(iterable.iterator()).isExhausted();
}

@Test
Expand All @@ -145,7 +145,7 @@ void ignoreNonSyntheticCoberturaFields() {
for (FieldProbe probe : iterable) {
fields.add(probe.getField());
}
assertThat(fields.size()).isEqualTo(1);
assertThat(fields).hasSize(1);
assertThat(fields.get(0).getName()).isEqualTo("i");
}

Expand Down
4 changes: 2 additions & 2 deletions ...va/nl/jqno/equalsverifier/internal/reflection/annotations/AnnotationCacheBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ void annotationsArrayParametersAreFoundOnClass() {
assertTypeHasAnnotation(AnnotationWithValuesContainer.class, annotation);

Set<String> annotations = new HashSet<>(annotation.properties.getArrayValues("annotations"));
assertThat(annotations.contains("javax.annotation.Nonnull")).isTrue();
assertThat(annotations.contains("nl.jqno.equalsverifier.testhelpers.annotations.NotNull")).isTrue();
assertThat(annotations).contains("javax.annotation.Nonnull");
assertThat(annotations).contains("nl.jqno.equalsverifier.testhelpers.annotations.NotNull");
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion equalsverifier-core/src/test/java/nl/jqno/equalsverifier/internal/util/ValidationsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// CHECKSTYLE OFF: IllegalImport

import static nl.jqno.equalsverifier.internal.testhelpers.Util.coverThePrivateConstructor;
import static org.assertj.core.api.Assertions.*;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.assertAll;

import java.util.ArrayList;
Expand Down