RFR: JDK-8274172: Convert JavadocTester to use NIO [v2]

[Pavel Rappo]

On Thu, 23 Sep 2021 17:15:46 GMT, Jonathan Gibbons <jjg 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.
>
> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review comments;
>   Move `JavadocTester.copyDir` to `ToolBox`; use FileVisitor code

Jon, thanks for modernizing JavadocTester! Before you read my suggestions, please consider this. I see a review as an opportunity to learn and improve, not just to mold a PR into a mainline commit.

In this particular case, I want to figure out the right way of copying a file tree. Why? Because I see opportunities of reusing this code in javadoc. For example, we have similar functionality in DocFilesHandlerImpl.copyDirectory. Why not spend extra time to figure out, if only a javadoc-internal, reference way of copying file trees?

test/langtools/tools/lib/toolbox/ToolBox.java line 270:

> 268:     public void copyDir(Path fromDir, Path toDir) {
> 269:         try {
> 270:             Files.createDirectories(toDir);

Do not create this directory manually. It will be created first thing in your visitor.
Suggestion:

test/langtools/tools/lib/toolbox/ToolBox.java line 283:

> 281:                                 if (!Files.isDirectory(toSubdir))
> 282:                                     throw e;
> 283:                             }

If we do not expect the target directory to ever exist, we should remove this try-catch. This would make us fail fast, rather than add something to the existing file tree silently. Also, this situation could only happen in the first call to this method, right?
Suggestion:

                            Files.copy(fromSubdir, toSubdir);

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

Changes requested by prappo (Reviewer).

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