On Wed, 11 Jun 2025 22:40:43 GMT, Justin Lu <j...@openjdk.org> wrote:

>> Please review this PR which finishes Applet removal for the test: 
>> jdk/internal/loader/URLClassPath/ClassnameCharTest.java.
>> 
>> `testclasses.jar` is updated such that the two classes no longer extend 
>> Applet.
>> 
>> 
>> $ javap fo\ o.class 
>> public class fo o {
>> }
>> $ javap æ$'\302\211'$'\302\213'å$'\302\206'$'\302\214'.class 
>> public class 手册 {
>> }
>> 
>> 
>> The bug description of 
>> [JDK-8358729](https://bugs.openjdk.org/browse/JDK-8358729) contains the 
>> original `javap` output for those classes.
>> 
>> Additionally, the security APIs that were marked for removal are also 
>> removed from this test as well.
>
> 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 31:

> 29:  * @modules java.base/sun.net.www
> 30:  *          jdk.httpserver
> 31:  * @compile -XDignore.symbol.file=true ClassnameCharTest.java

Hello Justin, this is pre-existing but since we are updating this test, I think 
it better to remove this `@compile` line altogether. It isn't needed for this 
test.

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);
}

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25703#discussion_r2143020492
PR Review Comment: https://git.openjdk.org/jdk/pull/25703#discussion_r2143023616

Reply via email to