All these tests specify this already so it doesn’t seem sufficient. -Phil.
> On Dec 7, 2019, at 12:07 PM, Igor Ignatyev <igor.ignat...@oracle.com> wrote: > > can we just add '@modules jdk.incubator.jpackage' to > test/jdk/tools/jpackage/junit/junit.java to solve that? or some of the tests > run by test/jdk/tools/jpackage/junit/junit.java don't need > jdk.incubator.jpackage module? > > -- Igor > >> On Dec 7, 2019, at 11:57 AM, Philip Race <philip.r...@oracle.com> wrote: >> >> Yes, since only a (relatively) small number of tests needed to be updated >> this is fine with me at least for now. So +1 >> >> -phil. >> >>> On 12/7/19, 5:48 AM, Andy Herrick wrote: >>> Phil - are you approving this change ? - I think you are the only >>> registered Reviewer. >>> >>> /Andy >>> >>>> On 12/6/2019 8:11 PM, Phil Race wrote: >>>> Well we could deprecate and remove the solaris port :-) >>>> But until that is done this is the only way I know of. >>>> we could require all jpackage tests to include some helper code which >>>> decides if it is applicable but that will be more work upfront and many >>>> jpackage tests are already platform specific so @requires is not going >>>> away. >>>> >>>> >>>> -Phil. >>>> >>>>> On Dec 6, 2019, at 2:33 PM, Alexander Matveev >>>>> <alexander.matv...@oracle.com> wrote: >>>>> >>>>> Looks good, but is there better way to exclude tests on Solaris? I do not >>>>> like idea adding @requires for all tests. >>>>> >>>>> Thanks, >>>>> Alexander >>>>> >>>>>> On 12/6/2019 10:35 AM, Alexey Semenyuk wrote: >>>>>> Looks good. >>>>>> >>>>>> - Alexey >>>>>> >>>>>>> On 12/6/2019 1:33 PM, Andy Herrick wrote: >>>>>>> Please review this jpackager test fix for bug [1] at [2]. >>>>>>> >>>>>>> the fix adds "@requires (os.family == "linux") | (os.family == "mac") | >>>>>>> (os.family == "windows")" to all jpackage tests that were not already >>>>>>> filtered with "@requires (os.family == "XXX")" >>>>>>> >>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8235453 >>>>>>> >>>>>>> [2] http://cr.openjdk.java.net/~herrick/8235453/ >>>>>>> >>>>>>> /Andy >>>>>>> >