RFR: JDK-8274172: Convert JavadocTester to use NIO

[Jonathan Gibbons]

On Thu, 23 Sep 2021 09:52:55 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

>> Please review a moderately simple update to convert JavadocTester to just use NIO, instead of a mix of File and NIO.
>> 
>> The original code used java.io.File. At some point (JDK 9-ish) new code was added that used NIO, resulting in a mix. This change converts the old code to use NIO as well.
>> 
>> This is mostly internal, with two changes that affect tests.
>> 
>> 1. The `protected` field `outputDir` is changed from a `File` to a `Path`.   Some tests use `outputDir` directly, typically to convert it to a `Path`.
>> 2. The `copyDir` method had a strange spec.  Partly, it used "target" to describe the directory being copied, but worse, it copied the entire source directory INTO the destination directory, as compared to copying the contents.  The method was just used in a single test, so I've changed the spec of the method and the use in the test.  This cleaned up a "TODO" as well, to use `Files.walkFileTree` for the copy.
>
> test/langtools/jdk/javadoc/doclet/testSearchScript/TestSearchScript.java line 73:
> 
>> 71:         Bindings bindings = engine.getBindings(ScriptContext.ENGINE_SCOPE);
>> 72:         bindings.put("polyglot.js.nashorn-compat", true);
>> 73:         engine.eval(Files.newBufferedReader(Path.of(testSrc).resolve("javadoc-search.js")));
> 
> Unrelated to this change per se: who closes this buffered reader?

That's unrelated, for @hns

> test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 802:
> 
>> 800:                 public FileVisitResult preVisitDirectory(Path fromSubdir, BasicFileAttributes attrs) throws IOException {
>> 801:                     Path toSubdir = toDir.resolve(fromDir.relativize(fromSubdir));
>> 802:                     if (Files.exists(toSubdir)) {
> 
> Shouldn't this check be negated? Otherwise, sub-folders are not copied. On the second thought, why do we need this check at all?

Oops.
Whether it is needed depends on whether you think the target directory exists or not.  In the one existing use case, the directory should not exist and so the  check is not necessary.   Given that tests run in an empty scratch dir, we could reasonably assume that is the case.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5644