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

[Mahendra Chhipa]

On Fri, 22 Jan 2021 17:59:33 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Mahendra Chhipa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   NonJavaName Tests updated
>>   Used newInstance() method to create the different EnclosingClasses at runtime
>
> test/jdk/java/lang/Class/forName/NonJavaNameTest.java line 49:
> 
>> 47:     private static final String SRC_DIR = System.getProperty("test.src");
>> 48:     private static final String NONJAVA_NAMES_SRC = SRC_DIR + "/classes/";
>> 49:     private static final String NONJAVA_NAMES_CLASSES = System.getProperty("test.classes", ".");
> 
> I was at first confused with `NON_NAMES_CLASSES` of what it means.    For simplicity and clarity, better to rename these variables:
> s/NONJAVA_NAMES_SRC/TEST_SRC/
> s/NONJAVA_NAMES_CLASSES/TEST_CLASSES/

Implemented in next patch

> test/jdk/java/lang/Class/forName/NonJavaNameTest.java line 43:
> 
>> 41:  * @run testng/othervm NonJavaNameTest
>> 42:  * @summary Verify names that aren't legal Java names are accepted by forName.
>> 43:  * @author Joseph D. Darcy
> 
> we can drop this `@author` in particular this is a new file.

Now this file is deleted.

> test/jdk/java/lang/Class/forName/NonJavaNameTest.java line 96:
> 
>> 94: 
>> 95:     @AfterClass
>> 96:     public void deleteInvalidNameClasses() throws IOException {
> 
> jtreg will do the clean up and this is not necessary.

Removed in next patch.

> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 95:
> 
>> 93:             Files.deleteIfExists(pkg2Dir);
>> 94:             Files.deleteIfExists(pkg1File);
>> 95:             Files.deleteIfExists(pkg1Dir);
> 
> You can consider using `jdk.test.lib.util.FileUtils` test library to remove the entire directory.

Using FileUtils in next patch

> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 97:
> 
>> 95:             Files.deleteIfExists(pkg1Dir);
>> 96:             Files.createDirectories(pkg2Dir);
>> 97:         } catch (IOException ex) {
> 
> The test should fail when running into any error.    Using the test library will do the retry

Thanks, Corrected in next patch.

> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 197:
> 
>> 195:                 c.getSimpleName(), annotation.simple(),
>> 196:                 c.getCanonicalName(),
>> 197:                 annotation.hasCanonical() ? annotation.canonical() : null);
> 
> I am guessing this whitespace in line 195-197 is not intentional?  Same for line 203-206?

Thanks, Removed in next patch. Trying to keep maximum line length 120.

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

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