On Mon, 21 Oct 2024 21:12:29 GMT, Phil Race <p...@openjdk.org> wrote:

>> Sean Mullan has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 97 commits:
>> 
>>  - Merge remote-tracking branch 'jdk-sandbox/jep486' into JDK-8338411
>>  - Change apiNote to deprecated annotation on checkAccess methods. Change 
>> method dedescription to "Does nothing".
>>  - Sanitize the class descriptions of DelegationPermission and 
>> ServicePermission
>>    by removing text that refers to granting permissions, but avoid changes 
>> that
>>    affect the API specification, such as the description and format of input
>>    parameters.
>>  - Restored methods in RMIConnection to throw SecurityExceptions again but
>>    with adjusted text that avoids the word "permission".
>>  - Add text to class description of MBeanServer stating that implementations
>>    may throw SecurityException if authorization doesn't allow access to 
>> resource.
>>  - Restore text about needing permissions from the desktop environment in the
>>    getPixelColor and createScreenCapture methods.
>>  - Add api note to getClassContext to use StackWalker instead and
>>    add DROP_METHOD_INFO option to StackWalker.
>>  - Change checkAccess() methods to be no-ops, rather than throwing
>>    SecurityException.
>>  - Merge
>>  - Merge
>>  - ... and 87 more: https://git.openjdk.org/jdk/compare/f50bd0d9...f89d9d09
>
> test/jdk/javax/imageio/CachePremissionsTest/CachePermissionsTest.java line 76:
> 
>> 74:         System.out.println("java.io.tmpdir is " + 
>> System.getProperty("java.io.tmpdir"));
>> 75: 
>> 76:         if (args.length > 1) {
> 
> The isFileCacheExpected logic does not make sense. The test sets set to use 
> the cache but then reads whether to expect it based on the args[0]. If that 
> were set to false  the test will fail. So why is it there ?
> 
> Also the messing around with exceptions at the end of the test is pointless

@prrace I might have missed removing this check which was in the original test. 
The latest update to this test has two run tags but it fails when 
isFileCacheExpected is set to true.

Did you mean to keep only one run tag? 
https://github.com/openjdk/jdk-sandbox/commit/1bf77a393c5756bca65760402077617d37be72d2

I'll be rename the test as suggested when I update this test next.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1813429265

Reply via email to