Skip to content

Add doesNotHaveToString assertion#2052

Merged
joel-costigliola merged 4 commits intoassertj:mainfrom
lykims:feature/doesNotHaveToString
Nov 30, 2020
Merged

Add doesNotHaveToString assertion#2052
joel-costigliola merged 4 commits intoassertj:mainfrom
lykims:feature/doesNotHaveToString

Conversation

@lykims
Copy link
Contributor

@lykims lykims commented Nov 26, 2020

Check List:

Minor fixes for hasToString doc description and ShouldHaveToString error as well.

Copy link
Member

@joel-costigliola joel-costigliola left a comment

Choose a reason for hiding this comment

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

@lykims thanks, good PR, just a few improvements to do and we are good.

Comment on lines +484 to +485
* assertThat(homer).doesNotHaveToString("Marge");
* </code></pre>
Copy link
Member

Choose a reason for hiding this comment

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

join the two lines

}

private ShouldNotHaveToString(Object actual, String other) {
super("%nExpecting actual's toString() not to be equal to:%n <%s>%nbut was:%n <%s>", other, actual.toString());
Copy link
Member

Choose a reason for hiding this comment

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

let's simplify this to:

"%nExpecting actual's toString() not to be equal to:%n  <%s>"

the reason is but was:%n <%s> section is actually repeating actual's toString value

// GIVEN
Object actualObject = null;
// WHEN
ThrowingCallable code = () -> objects.assertDoesNotHaveToString(someInfo(), actualObject, "bar");
Copy link
Member

Choose a reason for hiding this comment

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

use expectAssertionError from org.assertj.core.util.AssertionsUtil which returns an AssertionError

AssertionError error = expectAssertionError(() -> objects.assertDoesNotHaveToString(info, actual, "Person[name='foo']"));
// THEN
verify(failures).failure(info, shouldNotHaveToString("Person[name='foo']", "Person[name='foo']"));
assertThat(error).hasMessageContainingAll("Person[name='foo']");
Copy link
Member

Choose a reason for hiding this comment

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

remove this line, it is already covered by verify(failures) (we don't want to test the error message but only that we call the proper error message factory)

// GIVEN
AssertionInfo info = someInfo();
// WHEN
AssertionError error = expectAssertionError(() -> objects.assertDoesNotHaveToString(info, actual, "Person[name='foo']"));
Copy link
Member

Choose a reason for hiding this comment

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

remove AssertionError error local variable (see next comment why)

@lykims
Copy link
Contributor Author

lykims commented Nov 29, 2020

@joel-costigliola I addressed your comments. I also applied changes suggested for Objects_assertDoesNotHaveToString_Test to Objects_assertHasToString_Test. Let me know if I miss anything!

@joel-costigliola joel-costigliola added this to the 3.19.0 milestone Nov 30, 2020
@joel-costigliola joel-costigliola merged commit 1ae4284 into assertj:main Nov 30, 2020
@joel-costigliola
Copy link
Member

Integrated thanks for the good work @lykims!

@lykims lykims deleted the feature/doesNotHaveToString branch December 4, 2020 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add doesNotHaveSameHashCodeAs and doesNotHaveToString assertions

2 participants