Details
-
Bug
-
Status: Closed (View Workflow)
-
Minor
-
Resolution: Fixed
Description
This is a follow-up to JENKINS-17941, listing any remaining issues I still see with version 1.11 of the plugin. Note that I'm very grateful for the latest changes which have fixed all major issues, so I'm starting to also list rather minor / cosmetic issues here.
1) On the first succeeding test run, i.e. when there is no previous run to compute a difference to, the "TAP Test Result" text say something like "(129 failures )", i.e. there is a superfluous whitespace after "failures ".
2) In case of no parse errors like at [1] the anchor text on the "TAP Extended Test Results" page is empty. The source code looks like "<a name="parseErrors"></a> No parse errors found.". For consistency, I think it would still be nice to have the heading in there which is shown if there is a table with parse errors. I.e., I think the source code should look like "<a name="parseErrors"><h3>Parse errors</h3></a> No parse errors found.".
3) The two links say "TAP Test Result" and "TAP Extended Test Results", but I think both should either use singular ("Result") or plural ("Results").
4) Clicking on any of the links in the "All (Failed) Tests" table at [2] still results in error 404 for me.
5) At [3], the title says "629 files 9549 tests, 8968 ok, 129 not ok, 452 skipped, 0 Bail Out!.", but I think just of either the exclamation mark or the dot at the end of the sentence would be enough 
6) If you search for "t7401-submodule-summary.sh" on [3] you'll see that for failed tests the table still contains a lot of code that should not be part of the table. There are more example on that page, just search for "# test_" to find them. Similarly, I think the lines like "# passed all 7 test(s)" should also not be part of the table. But that should come for free if you just strip any commenting lines starting with "#" from the raw TAP output before processing it.
[1] http://mingwgitdevenv.cloudapp.net/job/mingwGitDevEnv-test/2/
[2] http://mingwgitdevenv.cloudapp.net/job/mingwGitDevEnv-test/2/tapTestReport/
[3] http://mingwgitdevenv.cloudapp.net/job/mingwGitDevEnv-test/2/tapResults/?
Attachments
Issue Links
- is related to
-
-
- Closed
-
Activity
Sebastian Schuberth
added a comment - Thanks a lot, I can confirm all issues were addressed. There are just some very minor cosmetic issues left which I do not think justify to reopening this issues. But for the sake of completeness I'll mention them here:
Regarding 2), the dot at the end of the sentence got lost: Instead of "No parse errors found." it now says "No parse errors found".
Regarding 3), I now think the choice should probably have been to use singular ("Result") because that's what Jenkins itself is using in the "Latest Test Result" link.
Regarding 4), the 404 is gone, but the table contains entries like "4 - - pretend we have a known breakage". Why are there two consecutive dashes? And the test number "4" is not very useful by itself without mentioning the test it belongs to ("t0000-basic.sh" in this example).
Sebastian Schuberth
added a comment - Thanks a lot, I can confirm all issues were addressed. There are just some very minor cosmetic issues left which I do not think justify to reopening this issues. But for the sake of completeness I'll mention them here:
Regarding 2), the dot at the end of the sentence got lost: Instead of "No parse errors found." it now says "No parse errors found".
Regarding 3), I now think the choice should probably have been to use singular ("Result") because that's what Jenkins itself is using in the "Latest Test Result" link.
Regarding 4), the 404 is gone, but the table contains entries like "4 - - pretend we have a known breakage". Why are there two consecutive dashes? And the test number "4" is not very useful by itself without mentioning the test it belongs to ("t0000-basic.sh" in this example). 2) Could you show me an example? Here I see only a <H?>Parse errors</H?> followed by No parse errors found, in the bottom of the TAP Test Results UI
3) Hmmm, well... haha, we can fix it again later while working on other issues 
4) I think the consecutives dashes were already there, but without the test number.
There was a block of code, where I was retrieving the test name. This block of code had a dumb bug, where I was doing something like return a == b ? bla bla + bla bla : bla bla... I don't recall exactly what was wrong, but it was never returning the desired value.
Now it always return:
<TEST_NUMBER> + SPACE + <TEST_DESCRIPTION>
Being that the SPACE + <TEST_DESCRIPTION> part is added iff the TEST_DESCRIPTION is not blank. And TEST_DESCRIPTION is equivalent to the TAP Test Result name/description.
IOW:
ok 1 - How's it going?
ok -> STATUS
1 -> TEST_NUMBER
- How's it going -> TEST_DESCRIPTION
I'm not entirely happy with this approach, since sometimes it returns simply the test_number. Feel free to file another issue for this, but I'll probably take a look on it only in 1.14, 1.15 (Friday release is 1.13).
Cheers, and thanks again for the feedback!
Sebastian Schuberth
added a comment - 2) That's exactly what I mean (you have to read the following text inside the quotes very carefully): Previously it was "No parse errors found." (there is a dot after "found") it now says "No parse errors found" (not dot after "found"). Yeah, I known I'm picky
4) I really do not care deeply about this as I prefer the "TAP Extended Test Results" anyway. So from my side let's just keep is as-is.
Sebastian Schuberth
added a comment - 2) That's exactly what I mean (you have to read the following text inside the quotes very carefully): Previously it was "No parse errors found." (there is a dot after "found") it now says "No parse errors found" (not dot after "found"). Yeah, I known I'm picky
4) I really do not care deeply about this as I prefer the "TAP Extended Test Results" anyway. So from my side let's just keep is as-is. 2) Ah! Got it, my bad, I changed that while fixing the other 'dot'
Let me know if you need anything included in 1.13. So far I've labeled two minor issues to be included in this release.
Fixed in 1.12