On Thu, 12 Jun 2025 15:14:37 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Justin Lu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Jai's review - dynamically create jar file
>
> test/jdk/jdk/internal/loader/URLClassPath/ClassnameCharTest.java line 46:
>
>> 44:
>> 45: public class ClassnameCharTest {
>> 46: private static final String JAR_PATH = Utils.TEST_CLASSES +
>> Utils.FILE_SEPARATOR + "testclasses.jar";
>
> I think it would be better to use `java.nio.file.Path` which is like:
>
>
> private static final Path JAR_PATH = Path.of(".").resolve("testclasses.jar");
>
> That way we don't have to reference the `Utils.TEST_CLASSES`. `Path.of(".")`
> will end up being the scratch directory of the test so jtreg can then retain
> this JAR file if the test fails for any reason.
Thanks, this is cleaner.
> test/jdk/jdk/internal/loader/URLClassPath/ClassnameCharTest.java line 138:
>
>> 136: }
>> 137: } catch (Exception _) {}
>> 138: throw new ClassNotFoundException(name);
>
> I think we should propagate the underlying cause too, to help debugging if it
> fails for whatever reason. So something like:
>
>
> catch (Exception e) {
> throw new ClassNotFoundException(name, e);
> }
Good point, also gave the other exception a more helpful message as well.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25703#discussion_r2143188388
PR Review Comment: https://git.openjdk.org/jdk/pull/25703#discussion_r2143188431
