From aca4862d1aa66f9a4fdd74eb7979e094c6389993 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 09:05:58 +0100 Subject: [PATCH 1/6] Enable trace logging in CI, remove experimental succession --- .github/workflows/build.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e91ae2b77..1565def19 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -43,12 +43,10 @@ jobs: os: [ ubuntu-latest ] build-script: [ ./mvnw ] java-version: [ 11, 12, 13, 14, 15, 16, 17, 18 ] - experimental: [ false ] include: - os: windows-latest build-script: './mvnw.cmd' java-version: 17 - experimental: true steps: - name: Checkout repository @@ -61,7 +59,6 @@ jobs: java-version: '${{ matrix.java-version }}' - name: Compile and run tests - continue-on-error: ${{ matrix.experimental }} run: >- ${{ matrix.build-script }} -B @@ -71,6 +68,7 @@ jobs: '-Dcheckstyle.skip=true' '-Dstyle.color=always' '-Dmaven.artifact.threads=100' + '-Pshow-test-logs-in-console' clean package - name: Annotate test reports From 04a3bb996c6e40b8339de10cc6d204dee01c7051 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 09:09:20 +0100 Subject: [PATCH 2/6] Temporarily disable parallel test execution in Surefire to make logs easier to understand --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1565def19..36a821fcb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -59,11 +59,11 @@ jobs: java-version: '${{ matrix.java-version }}' - name: Compile and run tests + # TODO: add back -T8C to run tests in parallel again once Windows is fixed. run: >- ${{ matrix.build-script }} -B -U - -T8C --no-transfer-progress '-Dcheckstyle.skip=true' '-Dstyle.color=always' From b36279cdd0cfe768343cdea18bcb021b8428049d Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 09:42:35 +0100 Subject: [PATCH 3/6] Include text logs for surefire reports --- .github/workflows/build.yml | 6 ++++-- pom.xml | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 36a821fcb..16051a807 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -83,7 +83,9 @@ jobs: if: always() with: name: tests-java-${{ matrix.java-version }}-${{ matrix.os }} - path: "**/target/surefire-reports/TEST-*.xml" + path: | + **/target/surefire-reports/*.xml + **/target/surefire-reports/*.txt retention-days: 5 - name: Archive Jacoco reports @@ -91,7 +93,7 @@ jobs: if: always() with: name: coverage-java-${{ matrix.java-version }} - path: "**/target/site/jacoco/jacoco-*.xml" + path: "**/target/site/jacoco/jacoco*.xml" retention-days: 5 report-test-results: diff --git a/pom.xml b/pom.xml index 9b5bfeaf9..5dbb301b7 100644 --- a/pom.xml +++ b/pom.xml @@ -236,10 +236,12 @@ + true true true true + true true From 02cd9040e3cd22d62c8504c7b4534ecb66171c5e Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 09:50:52 +0100 Subject: [PATCH 4/6] Include OS name in coverage report name --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 16051a807..ca3598ed3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -92,7 +92,7 @@ jobs: uses: actions/upload-artifact@v2 if: always() with: - name: coverage-java-${{ matrix.java-version }} + name: coverage-java-${{ matrix.java-version }}-${{ matrix.os }} path: "**/target/site/jacoco/jacoco*.xml" retention-days: 5 From 3f0f4af96f8737a97a263a96a95963153ab46c6f Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 10:11:43 +0100 Subject: [PATCH 5/6] Fix #30 - remove hardcoded path separators --- .../src/test/resources/logback-test.xml | 2 +- .../jct/paths/PathLocationManager.java | 71 +++++++++++-------- 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/examples/example-lombok/src/test/resources/logback-test.xml b/examples/example-lombok/src/test/resources/logback-test.xml index a4125bf5b..c8a7930b6 100644 --- a/examples/example-lombok/src/test/resources/logback-test.xml +++ b/examples/example-lombok/src/test/resources/logback-test.xml @@ -7,7 +7,7 @@ - + diff --git a/java-compiler-testing/src/main/java/io/github/ascopes/jct/paths/PathLocationManager.java b/java-compiler-testing/src/main/java/io/github/ascopes/jct/paths/PathLocationManager.java index 1c8525167..d25efb3da 100644 --- a/java-compiler-testing/src/main/java/io/github/ascopes/jct/paths/PathLocationManager.java +++ b/java-compiler-testing/src/main/java/io/github/ascopes/jct/paths/PathLocationManager.java @@ -30,16 +30,12 @@ import java.io.IOException; import java.lang.module.ModuleFinder; import java.lang.ref.Cleaner; -import java.nio.file.CopyOption; import java.nio.file.FileSystem; import java.nio.file.FileSystemNotFoundException; -import java.nio.file.FileSystems; import java.nio.file.FileVisitOption; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.StandardCopyOption; import java.nio.file.spi.FileSystemProvider; -import java.security.NoSuchProviderException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -284,11 +280,11 @@ public ClassLoader getClassLoader() { * @return the file, or an empty optional if not found. */ public Optional getFileForInput(String packageName, String relativeName) { - var relativePath = packageNameToRelativePath(packageName); + var relativePathParts = packageNameToRelativePathParts(packageName); for (var root : roots) { - var path = root.resolve(relativePath).resolve(relativeName); + var path = resolveNested(root, relativePathParts).resolve(relativeName); if (Files.isRegularFile(path)) { - return Optional.of(factory.create(location, path, relativePath)); + return Optional.of(factory.create(location, path, root.relativize(path).toString())); } } @@ -305,13 +301,14 @@ public Optional getFileForInput(String packageName, String relativeN * @return the file object for output, or an empty optional if no paths existed to place it in. */ public Optional getFileForOutput(String packageName, String relativeName) { - var relativePath = packageNameToRelativePath(packageName); - return roots .stream() .findFirst() - .map(root -> root.resolve(relativePath).resolve(relativeName)) - .map(path -> factory.create(location, path, relativePath)); + .flatMap(root -> { + var relativePathParts = packageNameToRelativePathParts(packageName); + var path = resolveNested(root, relativePathParts).resolve(relativeName); + return Optional.of(factory.create(location, path, root.relativize(path).toString())); + }); } /** @@ -322,9 +319,9 @@ public Optional getFileForOutput(String packageName, String relative * @return the file, or an empty optional if not found. */ public Optional getJavaFileForInput(String className, Kind kind) { - var relativePath = classNameToRelativePath(className, kind.extension); + var relativePathParts = classNameToRelativePathParts(className, kind.extension); for (var root : roots) { - var path = root.resolve(relativePath); + var path = resolveNested(root, relativePathParts); if (Files.isRegularFile(path)) { return Optional.of(factory.create(location, path, className)); } @@ -343,13 +340,14 @@ public Optional getJavaFileForInput(String className, Kind kind) * @return the file object for output, or an empty optional if no paths existed to place it in. */ public Optional getJavaFileForOutput(String className, Kind kind) { - var relativePath = classNameToRelativePath(className, kind.extension); - return roots .stream() .findFirst() - .map(root -> root.resolve(relativePath)) - .map(path -> factory.create(location, path, className)); + .flatMap(root -> { + var relativePathParts = classNameToRelativePathParts(className, kind.extension); + var path = resolveNested(root, relativePathParts); + return Optional.of(factory.create(location, path, className)); + }); } /** @@ -451,12 +449,12 @@ public Iterable list( Set kinds, boolean recurse ) throws IOException { - var relativePath = packageNameToRelativePath(packageName); + var relativePathParts = packageNameToRelativePathParts(packageName); var maxDepth = walkDepth(recurse); var results = new ArrayList(); for (var root : roots) { - var path = root.resolve(relativePath); + var path = resolveNested(root, relativePathParts); if (!Files.exists(path)) { continue; @@ -479,7 +477,7 @@ public Iterable list( } if (results.isEmpty()) { - LOGGER.trace("No files found in any roots for {}", relativePath); + LOGGER.trace("No files found in any roots for package {}", packageName); } return results; @@ -612,22 +610,39 @@ private void destroyClassLoader() { } private String pathToObjectName(Path path, String extension) { - var pathString = path.toString(); - return pathString - .substring(0, pathString.length() - extension.length()) - .replace('/', '.'); + assert path.getNameCount() != 0 : "Got an empty path somehow"; + + var parts = new ArrayList(); + for (var part : path) { + parts.add(part.toString()); + } + + // Remove file extension on the last element. + var lastIndex = parts.size() - 1; + var fileName = parts.get(lastIndex); + parts.set(lastIndex, fileName.substring(0, fileName.length() - extension.length())); + + // Join into a package name. + return String.join(".", parts); } - private String packageNameToRelativePath(String packageName) { + private String[] packageNameToRelativePathParts(String packageName) { // First arg has to be empty to be able to accept variadic arguments properly. - return String.join("/", PACKAGE_SPLITTER.splitToArray(packageName)); + return PACKAGE_SPLITTER.splitToArray(packageName); } - private String classNameToRelativePath(String className, String extension) { + private String[] classNameToRelativePathParts(String className, String extension) { var parts = PACKAGE_SPLITTER.splitToArray(className); assert parts.length > 0 : "did not expect an empty classname"; parts[parts.length - 1] += extension; - return String.join("/", parts); + return parts; + } + + private Path resolveNested(Path base, String[] parts) { + for (var part : parts) { + base = base.resolve(part); + } + return base.normalize(); } private Predicate hasAnyKind(Iterable kinds) { From 4221f09b34281eb571159d4eabbf364d3a8b5a41 Mon Sep 17 00:00:00 2001 From: Ashley Scopes <73482956+ascopes@users.noreply.github.com> Date: Wed, 11 May 2022 10:15:55 +0100 Subject: [PATCH 6/6] Revert logging verbosity changes for diagnostics --- .github/workflows/build.yml | 3 +-- examples/example-lombok/src/test/resources/logback-test.xml | 2 +- .../src/test/resources/logback-test.xml | 2 +- .../example-serviceloader/src/test/resources/logback-test.xml | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ca3598ed3..f1c1149ac 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -59,16 +59,15 @@ jobs: java-version: '${{ matrix.java-version }}' - name: Compile and run tests - # TODO: add back -T8C to run tests in parallel again once Windows is fixed. run: >- ${{ matrix.build-script }} -B -U + -T8C --no-transfer-progress '-Dcheckstyle.skip=true' '-Dstyle.color=always' '-Dmaven.artifact.threads=100' - '-Pshow-test-logs-in-console' clean package - name: Annotate test reports diff --git a/examples/example-lombok/src/test/resources/logback-test.xml b/examples/example-lombok/src/test/resources/logback-test.xml index c8a7930b6..a4125bf5b 100644 --- a/examples/example-lombok/src/test/resources/logback-test.xml +++ b/examples/example-lombok/src/test/resources/logback-test.xml @@ -7,7 +7,7 @@ - + diff --git a/examples/example-serviceloader-with-jpms/src/test/resources/logback-test.xml b/examples/example-serviceloader-with-jpms/src/test/resources/logback-test.xml index c8a7930b6..a4125bf5b 100644 --- a/examples/example-serviceloader-with-jpms/src/test/resources/logback-test.xml +++ b/examples/example-serviceloader-with-jpms/src/test/resources/logback-test.xml @@ -7,7 +7,7 @@ - + diff --git a/examples/example-serviceloader/src/test/resources/logback-test.xml b/examples/example-serviceloader/src/test/resources/logback-test.xml index c8a7930b6..a4125bf5b 100644 --- a/examples/example-serviceloader/src/test/resources/logback-test.xml +++ b/examples/example-serviceloader/src/test/resources/logback-test.xml @@ -7,7 +7,7 @@ - +