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]> 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. > > >
