On Mon, 25 Jan 2021 20:51:06 GMT, Mahendra Chhipa <github.com+34924738+mahendrachh...@openjdk.org> wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8183372 > > Mahendra Chhipa has updated the pull request incrementally with one > additional commit since the last revision: > > Implemented the review comments. test/jdk/java/lang/Class/forName/NonJavaNames.java line 37: > 35: * @bug 4952558 > 36: * @library /test/lib > 37: * @build NonJavaNames This `@build` can be dropped as this is done implicitly. test/jdk/java/lang/Class/forName/NonJavaNames.java line 63: > 61: private static final String TEST_SRC = SRC_DIR + "/classes/"; > 62: private static final String TEST_CLASSES = > System.getProperty("test.classes", "."); > 63: Path dhyphenPath, dcommaPath, dperiodPath, dleftsquarePath, > drightsquarePath, dplusPath, dsemicolonPath, dzeroPath, dthreePath, dzadePath; These variables should belong to the `createInvalidNameClasses` method. You can simply add `Path` type declaration in line 78-87. test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 90: > 88: Path enclosingPath = Paths.get(ENCLOSING_CLASS_SRC); > 89: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1"); > 90: Path pkg2Dir = Paths.get(SRC_DIR+"/pkg1/pkg2"); nit: space before and after `+` is missing test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 89: > 87: public void createEnclosingClasses() throws Throwable { > 88: Path enclosingPath = Paths.get(ENCLOSING_CLASS_SRC); > 89: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1"); Alternatively: Path pkg1Dir = Paths.get(SRC_DIR, pkg1); Path pkg2Dir = Paths.get(SRC_DIR, pkg1, pkg2); Path pkg1File = pkg1Dir.resolve("EnclosingClass.java"); Path pkg2File = pkg2Dir.resolve("EnclosingClass.java"); test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 84: > 82: public class EnclosingClassTest { > 83: private static final String SRC_DIR = System.getProperty("test.src"); > 84: private static final String ENCLOSING_CLASS_SRC = SRC_DIR + > "/EnclosingClass.java"; Suggestion: private static final Path ENCLOSING_CLASS_SRC = Paths.get(SRC_DIR, "EnclosingClass.java"); Use Path::toString to convert to a String where needed. test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 103: > 101: String javacPath = JDKToolFinder.getJDKTool("javac"); > 102: OutputAnalyzer outputAnalyzer = > ProcessTools.executeCommand(javacPath, "-d", > System.getProperty("test.classes", "."), > 103: SRC_DIR + "/EnclosingClass.java", SRC_DIR + > "/pkg1/EnclosingClass.java", SRC_DIR + "/pkg1/pkg2/EnclosingClass.java"); `File.separator` is different on Unix and Windows. Either replace `/` with `File.separator`. Or you can use `java.nio.file.Path` and call `Path::toString` to get the string representation. See above suggestion of declaring `static final Path ENCLOSING_CLASS_SRC`. So the above should use `ENCLOSING_CLASS_SRC` and `pkg2File` instead (calling toString) test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 108: > 106: > 107: @Test > 108: public void testEnclosingClasses() throws Throwable { better to throw specific exceptions for example `ReflectiveOperationException`. Same for the `@Test` below. test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 124: > 122: @AfterClass > 123: public void deleteEnclosingClasses() throws IOException { > 124: Path pkg1Dir = Paths.get(SRC_DIR + "/pkg1"); Suggestion: Path pkg1Dir = Paths.get(SRC_DIR, "pkg1"); ------------- PR: https://git.openjdk.java.net/jdk/pull/2170
