Thanks for the review, Jon. I agree the test is weird. I had to check it does the right thing with everything in the same directory.
I’ll rewrite it to use separate directories as you suggest. Hannes > Am 07.04.2020 um 19:32 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com>: > > Hannes, > > The test is weird, and it took me a while to understand what I think is going > on. This predates your changes, but is still very confusing. > > If I understand the code correctly, initModulesAndPackages creates both > packages and modules (which contain packages) in the same "src" directory. > The same "src" directory is then used as an argument for both the module > source path and source path, in different runs of javadoc. This is confusing, > at least to me, because you can put a module on the source path and have > javadoc recognize it as a module. But, it seems to work in the test because > the test is relying on the naming conventions for qualified module names, and > so it doesn't get confused when looking for packages or modules. But, I'm > mildly surprised javac/javadoc doesn't complain at the "packages" found on > the module source path. > > The test would be clearer if initModulesAndPackages wrote into two separate > directories, one for module source code and the other for package source > code. This would then affect/improve all the existing @Test methods by making > it clear which directory structure they are reading source code from. > > If this simple refactoring works, the review is Approved. Otherwise, I'd > like to understand the issues in more detail. > > -- Jon > > > > On 4/3/20 1:38 AM, Hannes Wallnoefer wrote: >> I realised there’s a simpler solution to the problem. Instead of using a Set >> to track packages with non-modular documentation we can just return the >> module name that matches our internal model in #checkLinkCompatibility. >> >> I also changed the JBS summary to „javadoc fails to link to docs with >> non-matching modularity“ so it covers both cases. >> >> New Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.01/ >> <http://cr.openjdk.java.net/~hannesw/8240169/webrev.01/> >> >> Hannes >> >> >>> Am 02.04.2020 um 16:06 schrieb Hannes Wallnoefer >>> <hannes.wallnoe...@oracle.com <mailto:hannes.wallnoe...@oracle.com>>: >>> >>> Please review: >>> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8240169 >>> <https://bugs.openjdk.java.net/browse/JDK-8240169> >>> Webrev: http://cr.openjdk.java.net/~hannesw/8240169/webrev.00/ >>> <http://cr.openjdk.java.net/~hannesw/8240169/webrev.00/> >>> >>> This patch allows using external documentation even if it doesn’t match the >>> external library in terms of modularity, i.e. non-modular documentation can >>> be used for an modular library and vice versa. Instead of showing an error >>> a warning is issued. There is still a warning if code and documentation do >>> not match, but we use the check to tweak reference lookup so that we are >>> still able to link to the appropriate documentation. >>> >>> I think that all relevant cases for combinations of modular and non-modular >>> code are covered by existing tests (some of which change with this patch >>> obviouly). >>> >>> TestLinkOptionWithAutomaticModule.java covers using a jar file as automatic >>> or unnamed module, both of which cases were already supported as of >>> JDK-8212233, so no changes there. >>> >>> TestLinkOptionWithModule.java covers all combinations of modular and >>> non-modular code and documentation. For the two tests that cover >>> non-matching combinations, I changed the expected return code to OK and >>> added expected output with the link HTML. The message about the mismatching >>> documentation is still there, but it is a warning instead of an error. >>> >>> I also added the JBS id to the @bug tag in TestLinkOptionWithModule.java as >>> well as in TestClassCrossReferences.java which is another test that >>> contains significant changes. >>> >>> Thanks, >>> Hannes >>
