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

[Derek Thomson]

Thanks for the feedback. I just found the JavadocTester code, thanks for
the pointer.

The problem I'm having is that the tests don't currently check to this
level of detail, or at least I can't find where they do. The closest I can
find is some checks of the links. So, firstly, no test fails. To change
that, I'd have to write a specific test to check these specific tags are in
place. It's going to be a bit weird to have a test just for this thead
element and nothing else at the same level of detail. Also, I'm a big
proponent of testing and even test driven coding, and I really find that
checks of the lowest level HTML and CSS are really just tests that the code
didn't change (i.e. you might as well just use 'diff'), and are almost
entirely redundant, just mirror the code too closely, and are incredibly
brittle e.g. breaking due to whitespace changes. I usually test this sort
of thing with a browser driven rendering test that demonstrates that the
end result actually looks and behaves as expected, regardless of the
underlying mechanisms used to achieve that.

All that said, I can definitely add a "CheckHeaders" test of some kind, it
just seems like an oddity right now. Unless you're trying to get coverage
from this time going forward by forcing everyone to write a new test until
better coverage is achieved, I can go along with that. I still have my
brittleness concerns, but a browser rendering test framework is a major
infrastructure investment too.

I'll start tinkering with a test until you can follow up here anyway.

Thanks again,
Derek.

On Wed, Mar 6, 2019 at 5:32 PM Jonathan Gibbons <jonathan.gibbons at oracle.com>
wrote:

> Hi Derek,
>
> Thanks for taking a look at this.
>
> The code itself looks reasonable, but I see you didn't provide anything in
> the way of a test.  There's two ways to look at that ..  either your change
> will break some existing tests, which will need to be fixed up, or it will
> not break any existing tests, indicating there is a lack of test coverage
> in this area, and so a test would be worth while. Either way, we should
> make sure there is test coverage for this change.
>
> Writing javadoc tests is pretty easy these days, using the "JavadocTester"
> framework.  Typically these days, for a case like this, I would expect to
> see a test generate a class with a few methods, perhaps using
> Toolbox.writeJavaFiles, then run it through javadoc, using
> JavadocTester.javadoc, and then call checkOutput to verify that the
> expected output has been generated.
>
> -- Jon
>
>
>
> On 03/06/2019 03:37 PM, Derek Thomson wrote:
>
> Hi all,
>
> I saw this bug and thought I could take a stab at it. Could someone review
> my change please?
>
> Webrev: http://cr.openjdk.java.net/~jcbeyler/8219691/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8219691
>
> Thanks,
> Derek.
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190307/240dff77/attachment.html>