On Mon, 10 Oct 2022 16:48:07 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Mark Powers has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Text Blocks
>> - long line
>
> test/jdk/javax/security/auth/PrivateCredentialPermission/Serial2.java line 29:
>
>> 27: * @summary PrivateCredentialPermission should not use local
>> variable to enable debugging
>> 28: * implementation-dependent class
>> 29: * @run main/othervm/policy=Serial.policy Serial2
>
> I think you can remove `main/othervm/policy=Serial.policy` (actually you can
> remove the whole `@run` line then).
> The policy argument causes the test to run with a SecurityManager enabled,
> and there isn't any reason that this test needs to do that AFAICT. Also that
> policy file is for other tests in this directory for accessing the file
> system or JAAS credentials, which you are not accessing in this test.
fixed
> test/jdk/javax/security/auth/PrivateCredentialPermission/Serial2.java line 58:
>
>> 56:
>> 57: // Deserialize input stream and create a new object.
>> 58: ObjectInputStream ois = new ObjectInputStream(is);
>
> Use try-with resources here so the input stream is automatically closed even
> if there are exceptions.
using try-with resources
> test/jdk/javax/security/auth/PrivateCredentialPermission/Serial2.java line 61:
>
>> 59: PrivateCredentialPermission pcp2 =
>> 60: (PrivateCredentialPermission)ois.readObject();
>> 61: is.close();
>
> Not necessary as BAIS.close() is a no-op.
removed
> test/jdk/javax/security/auth/PrivateCredentialPermission/Serial2.java line 79:
>
>> 77: } catch (Exception e) {
>> 78: e.printStackTrace();
>> 79: throw new SecurityException("Serial test failed");
>
> If you include e as the cause (2nd argument) of `SecurityException` then you
> don't need to print the stack trace on line 78.
fixed
-------------
PR: https://git.openjdk.org/jdk/pull/10206
