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