RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java
Alexandre (Shura) Iline
alexandre.iline at oracle.com
Sat Jul 2 01:20:38 UTC 2016
Pleas review the new version of the fix.
http://cr.openjdk.java.net/~shurailine/8158670/webrev.01/
I have executed the changed test successfully on linux, windows, mac os x and solaris.
Shura
> On Jun 29, 2016, at 10:43 AM, Alexandre (Shura) Iline <alexandre.iline at oracle.com> wrote:
>
>
>> On Jun 29, 2016, at 10:41 AM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>
>>
>> It's not like the test silently passes as the test still covers the cross-platform modules.
>> The way I view this is that the platform=specific modules are "optional" and we update the expected result by detecting their presence (or the not). It's not a hack or workaround, but rather an enhancement for the test to handle different images.
>
> Oh … that makes more sense. I mis-understood it originally.
>
> Let me fix it like this.
>
> Shura
>
>>
>> Just my .02,
>> Valerie
>>
>> On 6/29/2016 10:22 AM, Alexandre (Shura) Iline wrote:
>>>> On Jun 28, 2016, at 5:22 PM, Valerie Peng <valerie.peng at oracle.com> wrote:
>>>>
>>>>
>>>> One of the purpose of this test is to test the ordering (see the initial bug which this test is for: JDK-6997010).
>>>>
>>>> The original test already detects the OS and will skip certain providers accordingly.
>>>> Instead of splitting the test into multiple platform-specific tests, maybe we can keep the original test but add module-presence checking? Is there API available to query if certain module are present?
>>> ModuleFinder.ofSystem().find(String).
>>>
>>> We can have only the cross-platform modules listed in @modules and make the test to pass silently if the required platform-specific modules are not present.
>>>
>>> So, for example, on windows, if the test would be executed against an image which have no jdk.crypto.mscapi, the test will not run any checks and report pass.
>>>
>>> This would help to avoid the additional test creation, but it will add another silently passing test, which is less clean.
>>>
>>> Mandy?
>>>
>>> Shura
>>>
>>>> If yes, then we can leave out the platform-specific providers from the @modules line and skip the providers if either the OS does not match or the module is not present.
>>>>
>>>> If we can't query what modules are available, then we may have to think of something else.
>>>> Valerie
>>>>
>>>> On 6/27/2016 12:27 PM, Mandy Chung wrote:
>>>>> I’m including security-dev which would be a better list to review this test fix.
>>>>>
>>>>> Valerie,
>>>>> Does this test have to be order-sensitive? I think this test would be cleaner to make it order-insensitive and simply test the security provider initialization.
>>>>>
>>>>> See my comments below.
>>>>>
>>>>>> On Jun 27, 2016, at 8:21 AM, Alexandre (Shura) Iline <alexandre.iline at oracle.com> wrote:
>>>>>>
>>>>>> Hi.
>>>>>>
>>>>>> Please take a look on a suggested for for the java/lang/SecurityManager/CheckSecurityProvider.java test.
>>>>>>
>>>>>> The test in question depend on a list of modules, some of them are platform-specific. Listing all the dependencies in one test is causing the test to be skipped on every platform. In an offline conversation it was decided that it is better to split this tests into a few tests to declare the per-platform module dependencies.
>>>>>>
>>>>>> The bug: https://bugs.openjdk.java.net/browse/JDK-8158670
>>>>>> The suggested fix: http://cr.openjdk.java.net/~shurailine/8158670/webrev.00/
>>>>> The copyright header start year of the new tests should be 2016.
>>>>>
>>>>> I would suggest to make CheckSecurityProvide a platform-neutral test, i.e.,
>>>>> - drop @requires
>>>>> - make line 94-97 to ignore the platform-dependent provider if it’s present in the white list
>>>>>
>>>>> If we could make this test order-insensitive, it’d be cleaner to maintain a platform-neutral list of security providers and one list for the platform-dependent security providers for each platform. Just an idea.
>>>>>
>>>>> Mandy
>>>>>
>>
>
More information about the security-dev
mailing list