Mandy, thanks for review. I’ve updated patch http://cr.openjdk.java.net/~anazarov/8178323/webrev.01/webrev/ <http://cr.openjdk.java.net/~anazarov/8178323/webrev.01/webrev/>
See my comments inline > On 8 Apr 2017, at 03:18, Mandy Chung <mandy.ch...@oracle.com> wrote: > > >> On Apr 7, 2017, at 8:40 AM, Andrey Nazarov <andrey.x.naza...@oracle.com> >> wrote: >> >> Hi, >> >> Please review 3 negative tests for Jlink which tests new >> bind-services/suggest-providers feature. >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8178323 >> >> webrev: http://cr.openjdk.java.net/~anazarov/8178323/webrev.00/webrev/ > > Thanks for adding these tests. > > BindServices.java > > 155 Path dir = Paths.get("verboseNoop”); > > I suggest to rename “verboseNoop” to “verboseNoBind” fixed > > line 159-161: formatting nit: can you add spaces to align with the first > argument in line 158. Same comment to SuggestProviders.java line 180-182 > an 198-200. Fixed > > SuggestProviders.java > It may be better to rename “suggestNotProvider" to “noSuggestedProvider”. We already have test with name noSuggestProvider. Idea here was to specify real existing type which is not provider according to module-info.java. I’ve renamed to “suggestTypeNotRealProvider”. > > In the noOneUsesProvider test case, is m4 not observable? I think > jlink should fail with m4 not found. When —-suggest-providers is > specified with a service type, it’s a bug in the implementation that > does not report it. This should be renamed to “nonObservableModule” > instead. We should file a bug and include this test case in the JBS report. Actually there are two issues here: https://bugs.openjdk.java.net/browse/JDK-8178404 <https://bugs.openjdk.java.net/browse/JDK-8178404> https://bugs.openjdk.java.net/browse/JDK-8178405 > > Mandy
