RFR (XS) 8219691: method summary table head should be enclosed in <thead>

[Jonathan Gibbons]

On 3/11/19 3:02 PM, Derek Thomson wrote:
> The first that comes to mind is that it doesn't tell you if the string 
> was expected or not - i just says string found or not found, but not 
> the setting of the "expected" flag. So it might say "string not found" 
> but that was fine in the case where the string isn't expected. Until I 
> realized that, I was interpreting the failure output incorrectly in 
> many cases - since the test framework outputs all tests' output even 
> when just one test in a class fails. This explains why I initially 
> thought so many unrelated tests were failing!

OK, that's worth fixing. I'm generally big on using the words "expected" 
or "unexpected" in messages to make it abundantly clear. I think in this 
case, you have to read the text in conjunction with the passed/failed 
message, which I agree is less than optimal.

In other contexts I've used messages like:

  * passed: string found, as expected
  * passed: string not found, as expected
  * failed: unexpected string found
  * failed: expected string not found



>
> The next is making the diffs better, in one case I had to put the 
> expected and actual into files to find a tiny difference, and mess 
> around with diff, as I just couldn't see it by eye. This would 
> actually be impossible in the case of a whitespace difference perhaps. 
> This might not actually be do-able in all cases - I haven't thought 
> too much about it yet. I just know that "spotting the difference" 
> between expected vs actual could get painful.

Yes, I know this pain point.  I don't want to go all the way to 
including/providing a full diff library, but it might help to identify 
the position of the first difference in terms of line and column, and 
maybe the different characters. That would certainly be reasonably easy 
to do in the context of JavadocTester.

FWIW, I've done the "extract-to-files and use diff/meld" myself at 
times. Trailing whitespace at the end of a line can be particularly 
annoying!

>
> Despite all this complaining it wasn't that bad though, quite 
> straightforward - and I really appreciate the level of testing here, 
> and that it isn't easy to put in place.

The test base has definitely grown over the years; the basic framework 
was in place before I joined the team, but the framework has been 
rewritten and improved over the years, and the general policy of adding 
good tests for features and bug fixes helps. So, your thanks should go 
to everyone past and present who has helped here.

I guess you can't create bugs in OpenJDK yet, so I'll file 2 RFEs for 
the suggestions you have noted.

-- Jon


>
> I will sync and fix addContent, thanks for the heads-up there.
>
>
>
> On Mon, Mar 11, 2019 at 2:53 PM Jonathan Gibbons 
> <jonathan.gibbons at oracle.com <mailto:jonathan.gibbons at oracle.com>> wrote:
>
>
>     On 3/11/19 2:48 PM, Derek Thomson wrote:
>     > Jonathan - I have an update of this fix in
>     > http://cr.openjdk.java.net/~jcbeyler/8219691/webrev.01/
>     >
>     As a trivial matter, you will have a merge conflict when you sync
>     with
>     the master repo:  addContent just got renamed to just "add".
>
>     -- Jon
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190311/bdf68cff/attachment-0001.html>