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