On Fri, 22 Jan 2021 18:16:54 GMT, Mandy Chung <mch...@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/getEnclosingClass/EnclosingClassTest.java line 103:
>
>> 101: createAndWriteEnclosingClasses(enclosingPath, pkg2File,
>> "pkg1.pkg2");
>> 102:
>> 103: String javacPath = JDKToolFinder.getJDKTool("javac");
>
> You can use `jdk.test.lib.compiler.CompilerUtils` test library to compile the
> file in process.
I tried to use the CopmilerUtils to compile the file, But this utility is not
able to the dependencies of the class. Example it is not able to find the
common.TestMe class while try to compile the EnclosingClass.java.
> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 141:
>
>> 139: }
>> 140:
>> 141: private void createAndWriteEnclosingClasses(final Path source,
>> final Path target, final String packagePath) {
>
> s/packagePath/packageName/
>
> no need to declare parameters as `final`
Renamed the packagePath to packageName in next patch. To avoid the Checkstyle
warning added the final key word with parameters.
> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 147:
>
>> 145: while ((line = br.readLine()) != null) {
>> 146: if (line.contains("canonical=\"EnclosingClass")) {
>> 147: line =
>> line.replaceAll("canonical=\"EnclosingClass", "canonical=\"" + packagePath +
>> ".EnclosingClass");
>
> suggestion: declare `var className = packageName + ".EnclosingClass";` and
> replace those occurrance.
Thanks. Implemented the comment in next patch.
> test/jdk/java/lang/Class/getEnclosingClass/EnclosingClassTest.java line 155:
>
>> 153: bw.println(line);
>> 154: }
>> 155: } catch (IOException ex) {
>
> Should not swallow the error and instead, let it propagate and the test
> should fail.
Thanks. Corrected this in next patch.
-------------
PR: https://git.openjdk.java.net/jdk/pull/2170
