RFR: JDK-8187642: The consolidated repo test makefile disables CONCURRENCY setting to jtreg

[Erik Joelsson]

Hello,

On 2017-09-19 03:33, Magnus Ihse Bursie wrote:
> On 2017-09-19 03:05, David Holmes wrote:
>> Hi Erik,
>>
>> On 19/09/2017 10:53 AM, Erik Joelsson wrote:
>>> When invoking the test/Makefile using the jtreg_tests target in the 
>>> consolidated repo, the CONCURRENCY variable gets overridden with an 
>>> empty value. This was introduced with my changes to handle the new 
>>> test roots. I believe the proper fix is to surround the override of 
>>> CONCURRENCY with a conditional.
>>
>> Why isn't this handled by the preceding block:
>>
>>   51   ifeq ($(shell $(EXPR) $(JOBS) \> 50), 1)
>>   52     # JTReg cannot handle more than 50 in concurrency
>>   53     JDK_TEST_JOBS=50
>>   54   else
>>   55     JDK_TEST_JOBS=$(JOBS)
>>   56   endif
>>   57 else
>>   58   JDK_TEST_JOBS=$(TEST_JOBS)
>>   59 endif
>>
>
> At this point, it's probably not worth the effort to clean up the 
> logic in test/Makefile.
>
We were in quite a hurry getting this fix in before the first post 
consolidation promotion so I missed your comment before pushing, David. 
I agree in principle that the conditional should be made once instead of 
inlined in four places, but since Magnus is working actively to replace 
these makefiles in JDK 10, I don't think it's worth spending time trying 
to clean up minor details when the whole thing is in such a messy state.

/Erik
> /Magnus
>
>> David
>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8187642
>>>
>>> Webrev: http://cr.openjdk.java.net/~erikj/8187642/webrev.01/
>>>
>>> /Erik
>>>
>