RFR: JDK-8183372 : Refactor java/lang/Class shell tests to java [v3]

[Mandy Chung]

On Mon, 25 Jan 2021 20:51:06 GMT, Mahendra Chhipa <github.com+34924738+mahendrachhipa at 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