Infra: migrate to Truth in tests

[pbludov]

We agreed to use Truth in tests. It is more readable and produces better diffs on failure.

This is how a regular unit test should be written:

    @Test
    public void testFomeMethod() {
        assertWithMessage("Human-readable assertion")
            .that(someClass.method(arg1, arg2))
            .isTrue()
            .contains(...)
            .isInstanceOf(...)
    }

Assertion starts with assertWithMessage(...), followed by that and number of assertions.

For asserting exceptions:

    @Test
    public void testMethodThrowsException() {
        final SomeExceptionType exception =  assertThrows(SomeExceptionType.class, () -> {
            someClass.someMethod(args);
        });
        assertWithMessage("Invalid exception message")
            .that(exception)
            .hasMessageThat()
                .isEqualTo("The expected message");
        assertWithMessage("Invalid inner exception")
            .that(exception)
            .hasCauseThat()
                .isInstanceOf(...)
                .hasMessageThat()
                    ...;
    }

[nrmancuso]

@pbludov would truth be used for check unit tests? If so, would we deprecate usage of verify() and create custom assertion methods?

[pbludov]

@pbludov would truth be used for check unit tests? If so, would we deprecate usage of verify() and create custom assertion methods?

That would be perfect. We should try to implement such extensions.

[nrmancuso]

@pbludov since this issue is approved, should we begin using truth in all new unit tests going forward?

[romani]

Yes, we try to keep this in mind during review

[nrmancuso]

Yes, we try to keep this in mind during review

Should we create separate issue to do #9142 (comment) so that we can use this in check UT's sooner?

[nrmancuso]

Also, would this be a good GSOC 2nd issue? Break this issue into one for each test class?

[romani]

Yes, but we need to find test classes that need update, as majority of tests already abstracted a lot from core asserts, example 7190c47

It would be good to think how we can enforce no new files with junit assert.

[strkkk]

What do you think - should we repeat code several times or extract method to make such things as one-liner?
I mean code like
https://github.com/checkstyle/checkstyle/pull/10026/files#diff-cf838e5bbdb7de5a3e0e91f9919a389a0fd2ffdbda60c85a3c6dba07dcf69acdR365-R375

        assertWithMessage(message.toString())
                .that(missingViolations)
                .isEmpty();

        assertWithMessage(message.toString())
                .that(unexpectedViolations)
                .isEmpty();

        assertWithMessage(message.toString())
                .that(differingViolations)
                .isEmpty();

this can be extracted in one method and be like assertIsEmpty(message, collection)

What do you think?

[rnveach]

Questions to resolve the last of this issue:

1. Is it valid to convert assertSame to isEqualTo for truth? Same ensure it is the same memory location, while I assume equal to ensures the "equal" method returns true.
2. What is the valid way to convert assertThrows to Truth? It uses a lambda and probably has an encapsulation for the try/catch.

[romani]

Is it valid to convert assertSame to isEqualTo for truth?

https://truth.dev/api/latest/com/google/common/truth/Subject.html#isSameInstanceAs(java.lang.Object)
But most case that I see in our code, should be converted to isEquals (class type comparison).

What is the valid way to convert assertThrows to Truth? It uses a lambda and probably has an encapsulation for the try/catch.

yes, we already do this.

checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/utils/CommonUtilTest.java

Lines 122 to 128 in 84eb861

	final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> {
	CommonUtil.createPattern("[");
	});
	assertWithMessage("Invalid exception message")
	.that(ex)
	.hasMessageThat()
	.isEqualTo("Failed to initialise regular expression [");

[rnveach]

@romani

What is the valid way to convert assertThrows to Truth? It uses a lambda and probably has an encapsulation for the try/catch.
yes, we already do this.

The code you quoted uses assertThrows which is what I am asking how to convert and remove because as I understand this issue it should be removed. assertThrows is not a Truth assertion, and uses the forbidden library org.junit.jupiter.api.Assertions which we are trying to forbid and remove all uses of.

If it is allowed to stay, then we will have to make an allowance for it as it is currently being forbidden.

most case that I see in our code

Can point me to some case that should not be isEqualTo? I will start PR and we can review.
Edit: I did see one example that needed something different for assertNotSame.

[rnveach]

assertThrows

#9142 (comment)
Still waiting on a resolution for this.
This is the main thing left to handle for this issue. Next PR will most likely resolve everything.

[romani]

https://stackoverflow.com/a/38866403

No good API, let's keep assertThrows