Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Ok, please send me your putback comment. Otherwise, I will just use my own wording. ;) Should get it done either later today or tomorrow... Thanks, Valerie On 7/6/2016 11:31 AM, Alexandre (Shura) Iline wrote: Valerie, could you sponsor the patch for me? Shura On Jul 6, 2016, at 10:08 AM,

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Valerie, could you sponsor the patch for me? Shura > On Jul 6, 2016, at 10:08 AM, Valerie Peng wrote: > > > Changes look fine to me. > Thanks, > Valerie > > On 7/5/2016 2:31 PM, Mandy Chung wrote: >>> On Jul 5, 2016, at 1:53 PM, Alexandre (Shura) Iline >>>

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Changes look fine to me. Thanks, Valerie On 7/5/2016 2:31 PM, Mandy Chung wrote: On Jul 5, 2016, at 1:53 PM, Alexandre (Shura) Iline wrote: On Jul 5, 2016, at 1:36 PM, Mandy Chung wrote: On Jul 5, 2016, at 12:42 PM, Alexandre (Shura)

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jul 5, 2016, at 1:53 PM, Alexandre (Shura) Iline > wrote: > > >> On Jul 5, 2016, at 1:36 PM, Mandy Chung wrote: >> >> >>> On Jul 5, 2016, at 12:42 PM, Alexandre (Shura) Iline >>> wrote: >>> >>> This

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jul 5, 2016, at 1:36 PM, Mandy Chung wrote: > > >> On Jul 5, 2016, at 12:42 PM, Alexandre (Shura) Iline >> wrote: >> >> This made sense, than you, Mandy. >> >> Please review new version: >>

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jul 5, 2016, at 12:42 PM, Alexandre (Shura) Iline > wrote: > > This made sense, than you, Mandy. > > Please review new version: > http://cr.openjdk.java.net/~shurailine/8158670/webrev.02/ You can use Layer::findModule instead of Configuration::findModule.

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Jon, > On Jul 1, 2016, at 6:44 PM, Jonathan Gibbons > wrote: > > The test can also report which providers it is testing and/or skipping, so > that anyone checking the .jtr file can verify the behavior. I have added some debug output, pls take a look.

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

This made sense, than you, Mandy. Please review new version: http://cr.openjdk.java.net/~shurailine/8158670/webrev.02/ Shura > On Jul 2, 2016, at 3:26 PM, Mandy Chung wrote: > > >> On Jul 1, 2016, at 6:20 PM, Alexandre (Shura) Iline >>

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jul 1, 2016, at 6:20 PM, Alexandre (Shura) Iline > wrote: > > Please review the new version of the fix. > http://cr.openjdk.java.net/~shurailine/8158670/webrev.01/ This looks much better. Small comment: you can use Layer::findModule and also

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

The test can also report which providers it is testing and/or skipping, so that anyone checking the .jtr file can verify the behavior. Maybe it fails if expected modules or providers are not found. -- Jon On 06/29/2016 10:50 AM, Mandy Chung wrote: Valerie’s suggestion is a good one. The

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jul 1, 2016, at 6:20 PM, Alexandre (Shura) Iline > wrote: > > 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. I

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

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 > wrote: > >

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Valerie’s suggestion is a good one. The test will require a minimum image with cross-platform security providers to run the test while it can still verify other platform-specific providers. Mandy > On Jun 29, 2016, at 10:41 AM, Valerie Peng wrote: > > > It's not

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

> On Jun 29, 2016, at 10:41 AM, Valerie Peng 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

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

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

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

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

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

Copyrights fixed in place. Thank you, Mandy. Shura > On Jun 27, 2016, at 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

Re: RFR: 8158670: Fix @modules in java/lang/SecurityManager/CheckSecurityProvider.java

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,