testlib collections: Improve test names / assertion stack traces for collection test suites

[Marcono1234]

trafficstars

API(s)

com.google.common.collect.testing.*

For example MapTestSuiteBuilder or ListTestSuiteBuilder

How do you want it to be improved?

The collection test suites should make it easier to identify where in the user code the test suite was created / to which user test class it belongs, especially if an assertion fails in the test suite

Why do we need it to be improved?

There are two issues with the collection test suites:

• You have to specify a test suite name with named(String), however if you specify
  • a custom short name, then depending on where you run the test, you don't immediately understand which user class created the test suite (this differs between tooling / IDEs; in Eclipse there seems to be no indication which of the user's test classes created the test suite, in IntelliJ and Maven on console it does include the test class name in the UI / console output)
  • the name of the enclosing test class, then the test output becomes verbose because Guava seems to repeat the name for its nested tests, e.g. " [collection size: one]"
• if an assertion fails, the complete stack trace is only Guava code

Example

Take for example this Maven test failure from Gson:

[INFO] Running com.google.gson.JsonArrayAsListSuiteTest
Error:  Tests run: 404, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.445 s <<< FAILURE! -- in com.google.gson.JsonArrayAsListSuiteTest
Error:  com.google.common.collect.testing.testers.CollectionCreationTester.testCreateWithNull_unsupported[JsonArray#asList [collection size: one]] -- Time elapsed: 0.001 s <<< FAILURE!
java.lang.AssertionError: expected to throw NullPointerException
	at com.google.common.collect.testing.testers.ReflectionFreeAssertThrows.doAssertThrows(ReflectionFreeAssertThrows.java:101)
	at com.google.common.collect.testing.testers.ReflectionFreeAssertThrows.assertThrows(ReflectionFreeAssertThrows.java:61)
	at com.google.common.collect.testing.testers.CollectionCreationTester.testCreateWithNull_unsupported(CollectionCreationTester.java:58)
	at jdk.internal.reflect.GeneratedMethodAccessor96.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at junit.framework.TestCase.runTest(TestCase.java:177)
	at junit.framework.TestCase.runBare(TestCase.java:142)
	at junit.framework.TestResult$1.protect(TestResult.java:122)
	at junit.framework.TestResult.runProtected(TestResult.java:142)
	at junit.framework.TestResult.run(TestResult.java:125)
	at junit.framework.TestCase.run(TestCase.java:130)
	at junit.framework.TestSuite.runTest(TestSuite.java:241)
	at junit.framework.TestSuite.run(TestSuite.java:236)
	at junit.framework.TestSuite.runTest(TestSuite.java:241)
	at junit.framework.TestSuite.run(TestSuite.java:236)
	at junit.framework.TestSuite.runTest(TestSuite.java:241)
	at junit.framework.TestSuite.run(TestSuite.java:236)
	at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:90)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

Or generally how the test suites are used in Gson:

• JsonArrayAsListSuiteTest
• JsonObjectAsMapSuiteTest

Current Behavior

Luckily Maven includes the name of the test Java class it is executing (in the example JsonArrayAsListSuiteTest), but besides that:

• The test name is (mostly) a Guava defined test name: com.google.common.collect.testing.testers.CollectionCreationTester.testCreateWithNull_unsupported
• The complete stack trace is either Guava or JUnit, there is not a single mention of the Gson test class which created the test suite

You would have to notice the test suite name (in the example "JsonArray#asList") and then manually look for the user test class which creates a test suite with that name.

Desired Behavior

See "How do you want it to be improved?" above

Not completely sure how it could be improved; some ideas:

• Guava could get the caller of createTestSuite() and include its qualified class name in the test suite name (maybe?) That might be mostly relevant for Eclipse, see "Why do we need it to be improved?" above. Alternatively the user could do that themselves by creating an additional TestSuite with that qualified class name, wrapping the Guava test suite.
• Guava could get the caller of createTestSuite() and include it in the stack trace of assertion errors (possibly as dummy 'suppressed' or 'cause' exception)
• Guava could get the class name of the used TestContainerGenerator implementation and somehow include it in the test suite names or assertion errors (?)

These ideas are not ideal; any other ideas are welcome!

Concrete Use Cases

See "Example" section above

Checklist

• [x] I agree to follow the code of conduct.

• [x] I have read and understood the contribution guidelines.

• [x] I have read and understood Guava's philosophy, and I strongly believe that this proposal aligns with it.

[Marcono1234]

Maybe this really mostly affects Eclipse (where the user test class name does not seem to be shown), and not that much IntelliJ or Maven. So I could understand if you consider this not so important, or something Eclipse should improve.

Though of course if there is an improvement from which all IDEs / tools could benefit, that would be great as well.

[ben-manes]

fwiw, Eclipse won't show it in the UI for compactness, but you can get to it by double clicking the root item, which performs the Go To File action, and that will go to where the test suite was defined. The UI is a rendering of the junit xml, which you can also export, where the line item is the testsuite's name element.

[cpovirk]

Thanks.

Reporting of this through our internal Bazel test runner is also not great, so I likewise end up searching for the value passed to named :(

It might end up being worth trying one of the quick options you suggested, though it might also get annoyingly complicated to do so while ensuring compatibility with Android and GWT/J2CL (which we test under internally to verify behavior in those environments). My guess is that it wouldn't actually be too hard, but that worry is what keeps me from running out to try something.

Of course, the "right" solution is presumably for the testlib to be written in some style other than that of manual JUnit 3 suites....

[ben-manes]

Oh, I think I know why this problem seemed so weird to me and that I must be missing something. In the Gson samples, the test is returning the underlying Guava testlib suite directly. This relies on the reporter to decorate with the caller, which the JUnit 5 Eclipse runner will do and can execute v3/v4 tests. However, if you run using the JUnit 3 or 4 runners, per the Run Configuration, then you get only the suite's test name. Since you return Guava's directly, it does not include your own in the hierarchy. It's been a long time for me since the JUnit 3 days, but I believe it was pretty common to decorate suites to capture the class names at each nested level for debuggability. You could see if the following works for you,

public static Test suite() {
  var suite = new TestSuite(JsonArrayAsListSuiteTest.class.getSimpleName());
  suite.addTest(istTestSuiteBuilder.using(new ListGenerator())...)
  return suite;
}

I suspect that Eclipse is defaulting to JUnit5 for me because my suite is a mixture of 3-5 (+ TestNG), so it picks up the dependency. JUnit 5 can run older suites using its test engines, so those being on the classpath allows Eclipse to run it fine even though the build uses the older runners directly due to JUnit5 being quite buggy and leaking memory, making it too unstable for anything but the simplest usages.

[Marcono1234]

it was pretty common to decorate suites to capture the class names at each nested level for debuggability

Thanks, yes that is what I was thinking of as workaround, but the slight disadvantage is that this adds another layer of nesting in the test result UI. And usage of <test-class>.class.getSimpleName() makes the code a bit verbose, and error-prone in case IDE code completion suggests the wrong class name or you move the code.