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

[Jonathan Gibbons]

Derek,

You may have already found the answer, but to be clear ... if I were 
writing the test for this, I would call "javadoc" with some suitable 
input file, and then call "checkOutput" to verify that a fragment of 
golden text is present in the generated file, where the fragment is just 
the beginning of the table, perhaps starting at `<table>` and ending at 
the first `<tr>`.   You could also do (instead) a shorter one going from 
the end of the caption to the beginning of the row, ensuring there is a 
`thead` in between.

Some more notes:

  * You may have come across methods like "checkLinks", "checkHeaders",
    "checkAccessibility".  These are recent new methods that are
    typically applied to the output of all runs of javadoc, because
    these are more robust checks than hand-crafting specific tests. 
    They are worthwhile because they cover output that is generated in
    many different parts of javadoc.   In contrast, your fix is a fairly
    localized fix, and testing it once or twice will likely be enough.

  * The tests are somewhat brittle, but we have put a significant amount
    of effort into making them less brittle over the years. (If you want
    nightmares, go look at the tests as they were in the early days of
    OpenJDK!)

  * Browser testing is all well and good, and we do some of that too,
    but it is temporal and time-consuming.  It -is- important that we
    make sure that code doesn't change when doing other work, and using
    diff would be a "poor" (that's a British understatement) way to go
    because any change to the generated files would break the test.

  * Older tests use literal example code, but the signal-to-noise ratio
    for these files is often through the roof because of the required
    legal header. Newer tests tend to generate small examples on the
    fly, using library classes like ToolBox, ModuleBuilder and
    ClassBuilder. Unless you *really* need big example code, I would
    recommend using one of these generator classes instead of using
    literal example code.

-- Jon


On 3/7/19 10:41 AM, Derek Thomson wrote:
> I take it back, I just found the tests that *use* the framework in 
> another part of the tree. This is obviously what you meant!
>
> On Thu, Mar 7, 2019 at 10:32 AM Derek Thomson <dthomson at google.com 
> <mailto:dthomson at google.com>> wrote:
>
>     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 <mailto: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/
>>         <http://cr.openjdk.java.net/%7Ejcbeyler/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/5fdf5dd5/attachment-0001.html>