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.il...@oracle.com> wrote: > > >> On Jun 29, 2016, at 10:41 AM, Valerie Peng <valerie.p...@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.p...@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.il...@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 >>>>> >> >
