RFR (XS) 8219691: method summary table head should be enclosed in <thead>
Jonathan Gibbons
jonathan.gibbons at oracle.com
Thu Mar 7 22:01:50 UTC 2019
All[*] the javadoc tests should always pass ... [*] but note that some
tests may be listed in test/langtools/ProblemList.txt, so make sure
you're taking that file into account when you run the tests. If you're
using the makefiles to run the tests, that should happen automatically.
That being said, there is one with sometimes-problematic behavior,
TestRedirectLinks.java, which requires access to public internet, but
which is skipped if that is not available. When run on Oracle's internal
systems, it typically does not have external access, and so does not
always test all the internal functionality, meaning that it may fail on
external systems if it has become broken without us noticing.
If you are seeing tests that fail, I suggest you discuss them here
first, before embarking on any additional campaign to get them working.
Given the number of CI systems building and testing OpenJDK on all
platforms, I would be very surprised to hear of tests failing in an
unmodified repo.
If you are seeing tests fail, and you are fixing them to pass, that
lessens the need to write a new test. But note that the general OpenJDK
practice is that as a general rule, one or more tests must be tagged
with the bug number in the @bug line, so that "infrastructure" can check
that a test was provided for a bug. (There's an exception mechanism
involving labels on bugs, that would not apply in this case.)
-- Jon
On 3/7/19 1:07 PM, Derek Thomson wrote:
> Thanks Jonathan,
>
> I think these test are fine now I've worked with them a bit - I just
> didn't want to build all this just to test my one case, basically.
> Anyway, a bunch of tests do fail for this change and I'm working
> through them.
>
> Another question: are all these tests expected to pass right now? It
> seems like some fail in ways that have nothing to do with my change.
> If not, I can produce another webrev for a fix for them next.
>
> -- Derek.
>
>
> On Thu, Mar 7, 2019 at 11:25 AM Jonathan Gibbons
> <jonathan.gibbons at oracle.com <mailto:jonathan.gibbons at oracle.com>> wrote:
>
> 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/dcfff3d9/attachment-0001.html>
More information about the javadoc-dev
mailing list