On Thu, 23 Sep 2021 01:44:27 GMT, Jonathan Gibbons <[email protected]> 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.
Changes requested by prappo (Reviewer).
test/langtools/jdk/javadoc/doclet/testRelativeLinks/TestRelativeLinks.java line
208:
> 206: Path f = outputDir.resolve(file);
> 207: out.println("touch " + f);
> 208: try (OutputStream fos = Files.newOutputStream(f)) {
Consider using `var` here.
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?
test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 795:
> 793: */
> 794: public void copyDir(Path fromDir, Path toDir) {
> 795: out.println("Copying " + fromDir + " to " + toDir);
I was surprised to find out there's no convenience method in java.nio.file for
copying file trees. In its absence we could maybe use something closer to the
second usage example in FileVisitor?
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?
test/langtools/jdk/javadoc/lib/javadoc/tester/JavadocTester.java line 1115:
> 1113: // sort the log entries because the subtests may not be
> executed in the same order
> 1114: tests.sort((a, b) -> a.compareTo(b));
> 1115: try (BufferedWriter bw =
> Files.newBufferedWriter(Path.of("tester.log"))) {
Consider using `var` here too.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5644
