RFR: 8354327: Rewrite runtime/LoadClass/LoadClassNegative.java
Mikhail Yankelevich
myankelevich at openjdk.org
Thu Apr 17 16:30:11 UTC 2025
On Wed, 16 Apr 2025 12:34:28 GMT, Anton Artemov <duke at openjdk.org> wrote:
> Rewrote the test runtime/LoadClass/LoadClassNegative so that it creates a dummy jar file on the fly and then removes it. Previously existing dummy.jar removed.
Thank you for your changes, just a few comments.
Could you please also change the copyright date? ` * Copyright (c) 2013, 2023, Oracle and/or its affiliates. All rights reserved.` -> ` * Copyright (c) 2013, 2025, Oracle and/or its affiliates. All rights reserved.`
Thank you for the changes. Looks good to me now, however please await for a reviewer to review and approve the code.
test/hotspot/jtreg/runtime/LoadClass/LoadClassNegative.java line 43:
> 41:
> 42: public static void main(String args[]) throws Exception {
> 43: final String filename = System.getProperty("test.src") + File.separator + "dummy.jar";
Please use the scratch directory for the temporary file creation.
https://openjdk.org/jtreg/faq.html#scratch-directory
test/hotspot/jtreg/runtime/LoadClass/LoadClassNegative.java line 44:
> 42: public static void main(String args[]) throws Exception {
> 43: final String filename = System.getProperty("test.src") + File.separator + "dummy.jar";
> 44: String bootCP = "-Xbootclasspath/a:" + filename;
I'd move it under line 48, closer to the place where it's used. I think the code will be easier to read this way, it took me a few read throughs to realise that this is related to line 51, and not to any of the lines before.
What do you think?
test/hotspot/jtreg/runtime/LoadClass/LoadClassNegative.java line 46:
> 44: // Create a dummy file in the scratch directory
> 45: final String filename = "dummy.jar";
> 46: File dummyFile = new File(filename);
Nitpick: I think changing this
final String filename = "dummy.jar";
File dummyFile = new File(filename);
to
Suggestion:
File dummyFile = new File("dummy.jar");
will simplify the test even further. It's fine as is, so unless there is another revision requested by other reviewers there is no need in this change imo
-------------
PR Review: https://git.openjdk.org/jdk/pull/24688#pullrequestreview-2772529755
PR Review: https://git.openjdk.org/jdk/pull/24688#pullrequestreview-2772949228
PR Review Comment: https://git.openjdk.org/jdk/pull/24688#discussion_r2046936832
PR Review Comment: https://git.openjdk.org/jdk/pull/24688#discussion_r2046941761
PR Review Comment: https://git.openjdk.org/jdk/pull/24688#discussion_r2047184763
More information about the hotspot-runtime-dev
mailing list