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 <[email protected]
<mailto:[email protected]>> 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
<[email protected] <mailto:[email protected]>>
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.
