RFR: Additional tests for overlapping packages.

[Alexandre Iline (Shura)]

Alan, I have update the fix with these changes:

1. For error text verification, whenever appropriate, I am using regex, such as this "in both module m. and m.”. This, obviously is a weaker check, as it allows invalid outputs such as "in both module m2 and m2” and others, but I believe it is acceptable for this case. If there will be more cases similar to this, I would create a library method later, which would do a stricter check. 

2. Removed the testOverlapUpgradePath for now.

3. Fixed the summary.

5. Fixed spaces and tabs - awfully sorry about that.

I have also added one more case into OverlappingExportedPackagesTest.

Shura
On Oct 1, 2015, at 6:23 PM, Alan Bateman <Alan.Bateman at oracle.com> wrote:

> 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.