Test reporting name #325
Test reporting name #325
Conversation
| name = name.substring(0, index); | ||
| } | ||
| } | ||
| String name = id.getLegacyReportingName(); |
There was a problem hiding this comment.
This breaks being able to set custom names in annotations for things like ParameterizedTests and would cause the xml rendering for junit5 in bazel to be significantly different from how it's done in maven and gradle. The comparative folder has all the xml that those tools generate. It's definitely less than ideal that one can have custom names that break uniqueness and defaults for certain types like Repeated and Parameterized in junit5 are not descriptive enough. That said those (the junit5 Repeated/Parameterized) can be set via name formatters in junit5.
There was a problem hiding this comment.
I wonder if it would make sense to have a flag you could set that gets checked on line 38 and fall backs into the else block if so.
There was a problem hiding this comment.
You're right — this would break the custom name logic.
Just as an FYI, we've been running this code for the past six months. One aspect we appreciated about LegacyReportingName is that it produces deterministic test names, which greatly simplifies tooling. In contrast, with custom names, developers have many ways to make test names non-deterministic, which complicates things.
Some developers did raise concerns about not being able to set custom names. We acknowledged this and decided to allow custom names locally but not in CI. So, it might be best to enforce this behavior via a system property check.
There was a problem hiding this comment.
JUnit5 has both the https://docs.junit.org/snapshot/user-guide/#junit-platform-reporting-open-test-reporting and https://docs.junit.org/snapshot/user-guide/#junit-platform-reporting-legacy-xml options for XML reporting. rules_jvm tries to match the default junit4 test runner's xml structure in bazel while closely following gradle/maven in terms of how they do junit5 xml. Seems like having some sort of flag you can set in a .bazelrc file could be cool that would change the reported naming since custom naming is pretty inconsistent and makes metrics gathering on test times more difficult. I can take a stab at doing something here, but I'm not a maintainer or anything so we'd need to get feedback from one of them.
2 participants
fixes: #319
The TestCaseXmlRenderer class reports displayName for tests marked as dynamic. In JUnit 5, all tests of typem TestTemplateDescriptor are considered dynamic, including tests annotated with @ParameterizedTest, @testtemplate, @testfactory, and @RepeatedTest. Assigning a displayName to these tests is optional, and there is no enforcement of unique display names.
To ensure consistent and unique test identification across tools, use the legacy reporting name instead of displayName for test reporting: