On Sat, 14 May 2022 00:30:00 GMT, Tim Prinzing <[email protected]> wrote:
> The Class::forName behavior change to match JNI FindClass is a compatible
> change and seems pretty attractive as it would be expected that
> Class::forName would give the same behavior as FindClass which uses the
> system classloader. The test for 8281006 was enhanced to test for this
> change. Merged master to pick up fixes to unrelated test failures to reduce
> noise.
`Class::forName(String)` javadoc should specify which class loader to use when
invoked with null caller similar to the other APIs you fixed for null callers.
I think this new test case does not belong to `NullCallerGetResource.java`
which is a test for `Module::getResource`. It's better to be placed under
the `test/jdk/java/lang/Class/forName` directory that makes it clear it's a
test for `Class::forName`.
Alternatively, we could move all the tests for null caller under a new
clearly-named directory, maybe `test/jdk/jdk/nullCaller`. This may allow to
do some refactoring and remove the duplicated code in these test cases. What
do you think?
src/java.base/share/classes/java/lang/Class.java line 385:
> 383: ClassLoader loader = (caller == null) ?
> 384: ClassLoader.getSystemClassLoader() :
> 385: ClassLoader.getClassLoader(caller);
Formatting nit:
ClassLoader loader = (caller == null) ? ClassLoader.getSystemClassLoader()
: ClassLoader.getClassLoader(caller);
test/jdk/java/lang/module/exeNullCallerGetResource/NullCallerGetResource.java
line 28:
> 26: * @test
> 27: * @bug 8281006
> 28: * @bug 8281001
Suggestion:
* @bug 8281006 8281001
`@bug` can have one or more bug IDs
-------------
PR: https://git.openjdk.java.net/jdk/pull/8711
