On Tue, 19 Jan 2021 16:14:51 GMT, Alan Bateman <al...@openjdk.org> wrote:
>>> >>> >>> This issue requires a Reviewer from someone working in this area. Please do >>> not sponsor or integrate until that review has been done. >> >> Ok, increased the number of required reviewers to 2. >> Hope that was the right move, as I don't see any other way to undo >> /integrate. > > Finally getting back to this. The update to ModulePatcher.java is good. Test > coverage needs discussion. There are four scenarios where test coverage is > lacking: > > 1. automatic module on the module path, patched to override or augment > classes/resources > 2. automatic module on the module path, patched to add new packages > 3. automatic module as the initial module, patched to override or augment > classes/resources > 4. automatic module as the initial module, patched to add new packages > > The patch adds automatic/PatchTest.java so it's adding test coverage for 4. > We should probably rename it to leave room for the other tests, or else > create it so that additional test coverage can be added. I assume the test > was copied from another test as there are a few left overs, e.g. `@modules > java.script` but the test does not use this module. I don't want to expand > the scope of this PR too much but I think we should at least cover 3 and 4 in > the test. Thanks @AlanBateman, I now implemented a few more tests. They should cover all four cases you described. ------------- PR: https://git.openjdk.java.net/jdk/pull/2000
