On 23/09/2015 15:45, Alexandre (Shura) Iline wrote:
Hi.
This change adds a few more cases to existing cases for overlapping packages:
http://cr.openjdk.java.net/~shurailine/webrev.overlapping.packages.2/
Sorry for the late reply, too many things going on. I have a few comments:
1. I agree it's good to check the output in addition to checking for a
non-0 exit. However it turns out to be fragile: it might be "in both
module m1 and m2" on one system and "in both module m2 and m1" on
another. This is because the system module path varies by platform and
build configuration.
2. testOverlapUpgradePath assumes that the upgrade module path can be
used as an alternative to the application module path. There are several
discussion points around the upgrade module path but I don't expect it
can be used to do anything other than upgrade modules that are on the
system module path (or linked into the runtime image). So maybe this one
should be dropped for now.
3. The @summary in OverlappingPackagesTest should be used as the updated
test includes positive scenarios now.
4. I'm sure that that OverlappingExportedPackagesTest is the best name
for this test because it is testing several negative scenarios that are
specified by Configuration.resolve. I think we've already got each of
these cases covered in ConfigurationTest but good to have them tested at
startup for the boot layer too.
5. There are trailing spaces and tabs in the patch.
-Alan.
