Bugfix: do not set TESTMODULES environment variable if JDK is not modular

[Zdeněk Žamberský]

On 02/06/2018 05:36 PM, Jonathan Gibbons wrote:
> Jiri, Mila,
>
> I wonder why the fix is necessary.
>
> TESTMODULES contains a property of the test description (the @modules 
> entry).  The env variable will not be set if the value is empty.
>
> So, given that (versions of) test suites are generally specific to a 
> version of the JDK, I wonder why you are trying to run tests with 
> @module entries on a non-modular JDK.
>
> And, if there is a plausible test case for such tests, the fix is 
> arguably incomplete, because the TEST* environment variables for shell 
> tests mirror the test.* system properties for Java tests.
>
> I think a better direction would be to warn against the use of 
> @modules on a non-modular JDK.
>
> -- Jon

... this reply surprised me a bit

Actually we have big jtreg test suites, which we run on various JDKs
(both modular and non-modular). Most of these are java jtregs,
where we fixed lot of them to work on both modular and non-modular JDKs,
by adding @modules tag. But some of these tests use shell scripts
(various special cases), so being able to infer modularity of JDK from
environment variables in these (shell jtreg tests), would be very 
convenient.

We definitely do not favor duplication of tests for modular JDK...

>
> On 2/6/18 2:55 AM, Jiri Vanek wrote:
>> On 12/20/2017 05:35 PM, Jiri Vanek wrote:
>>> On 12/15/2017 11:37 AM, Miloslav Zezulka wrote:
>>>> This is a very small patch fixing the issue pointed out in 
>>>> $subject. I've
>>>> added checks for both compile and test JDK - is this an overkill, 
>>>> since
>>>> most of the time the implication goes COMPILEJDK.modular =>
>>>> TESTJDK.modular, or a necessary thing to have for foolproofness?
>>>
>> ping?
>>>
>>> Works fine. Johnatan, can you push it? Or do you wont mila to create 
>>> regular webrev? Is there some place to add test for it?
>>>
>>> Tahnx a lot!
>>>    J.
>>>
>>>
>>
>>
>

best regards,

-- 
Zdeněk Žamberský
OpenJDK QE
Red Hat