[SUREFIRE-1140] Add anchors to test case names in Surefire reports. #85
Conversation
|
My little objection is the parameter introduced in the plugin. Was it necessary? I would rather not see it either. Why did you commit |
|
@trenchguinea |
|
I added the parameter in case any consumer was making assumptions about the format of the HTML and didn't want the extra markup. That said, I am not opposed to removing the opt-in feature of the anchoring since it complicates the code ever so slightly and does not change the display of the report in any way whether the anchoring is in place or not. However, I'll need to see if hard-coding the anchoring will break other existing unit tests since they're looking for very specific HTML in the report, and that HTML did not have anchoring on the test cases. Regarding the TEST XML file, I was just following the pattern set in place by the other unit tests that run the report on that XML file and then search for specific HTML to assert against. While it didn't have to be that XML file, it was easier than creating something from scratch. Finally, regarding your test and why it might not be working for you, I'll build it locally again and give it a try. I remember it was working for me at the time, but I might not have been using the report-only goal from the command-line as you are. |
|
It worked for me, though doing exactly what you did didn't work because it wasn't running the snapshot version. If I just did However, when I ran |
|
I used |
|
It's working for me. I got the anchor In case of publishing this feature we would need to have some hint or description on how our customers may externally link to individual test cases. The best would be to describe this feature in *.vm site files in maven-surefire-plugin/src/site/apt, for instance in usage.apt.vm. Can you pls remove the parameter in next commit? Thx |
|
Btw, anchor on test method is not a test case TC but test. |
|
I'm working on these suggestions now. |
…n regarding the anchor name formats.
|
Suggested changes are committed. Regarding whether a test method is a "test case" or a "test", they are largely used interchangeably. In the code they are referred to as "test cases". Specifically, the anchor is created around the "testCase.getName". Likewise, in the Surefire report itself they are under a heading called "Test Cases". However, most people just call them tests. As for why I had to put "TC_" (or something) in the front of the anchor name and not just |
|
@trenchguinea Thx, I will have a look. |
|
@trenchguinea |
|
I agree that the lack of a dot is annoying and very likely unintentional by the original author. I'm not personally aware of anyone linking to suites in the report, but my visibility into such matters is obviously limited. What I can say is that we wanted to start linking to them, in addition to the individual test cases themselves, which is what led to me adding this enhancement, so I wouldn't be surprised if someone out there in the world was doing so. On the other hand, now that we're making it an "official" capability through documentation it would be nice to get it right. Either way I can add wording to the .apt to explain either the inconsistency (due to passivity) or the break in passivity if we think it would be helpful. I considered it but decided it might just be superfluous. |
|
I will create an issue with missing "." dot after this PR was pushed to master. IMHO the fix would be doable in 3.0 with no arguments against it because we can break some behavior in major release. |
|
@trenchguinea I will push two simple fixes within the week and I will test this PR right after. |
|
LGTM |
|
@trenchguinea Pls close this PR. It was merged with master. |
No description provided.