> On Jan 30, 2019, at 6:48 AM, Magnus Ihse Bursie 
> <magnus.ihse.bur...@oracle.com> wrote:
> 
> 
> 
> On 2019-01-29 17:55, Erik Joelsson wrote:
>> Hello,
>> 
>> On 2019-01-29 03:12, Magnus Ihse Bursie wrote:
>>> If you are going down this road, you'll need to verify that JCOV_IMAGE_DIR 
>>> is not empty in RunTests. So keep the ifneq statement, but with an $(error 
>>> ...) instead. Otherwise we'll just fail spectacularly and incomprehensible 
>>> later on.
>> 
>> This has already gone in with my blessing, but to address your concern.
>> 
>> I don't see why we need to check that it's set in RunTest.gmk? In the local 
>> case, we set JCOV_IMAGE_DIR in spec.gmk so it's guaranteed to be set, 
> 
> ... but only if we configure with --with-jcov.
> 
> I'd just like the system to present a comprehensible error message if the 
> user does something like this: "make test TEST=tier1 TEST_OPTS=JCOV=true", 
> but has not configured using --with-jcov. Prior to this patch, we ran the 
> tests but without coverage. This is probably not what was intended, but it 
> didn't crash. Now we're going to get a malformed command line.
> 
> "Error: Unable to access jarfile /lib/jcov.jar" is not a very helpful error 
> message.
> 
> When I test now, something is odd with the "jcov-image" target as well. Even 
> if you have not run configure with --with-jcov, the target seemingly runs and 
> succeeds, but no jcov image is generated. Expected behavior would be an error 
> message stating that you need to use --with-jcov.

Do you have the exact command to reproduce that?

> 
> /Magnus
> 
>> and in the prebuilt case, it's the responsibility of RunTestPrebuilt.gmk to 
>> adjust things for the lack of a proper spec.gmk and in RunTestPrebuilt.gmk 
>> we already check that JDK_IMAGE_DIR points to an existing directory.
>> 
>> /Erik
>> 
>>> /Magnus
>>> 
>>>> 28 jan. 2019 kl. 18:47 skrev Erik Joelsson <erik.joels...@oracle.com>:
>>>> 
>>>> Hello Shura,
>>>> 
>>>> In RunTest.gmk, you removed the conditional, but the comment still implies 
>>>> that it's there. Please update the comment. Otherwise this looks good!
>>>> 
>>>> /Erik
>>>> 
>>>>> On 2019-01-25 15:20, Alexandre (Shura) Iline wrote:
>>>>> Hi,
>>>>> 
>>>>> Please take a look on a change to allow JCov test execution through jib.
>>>>> 
>>>>> Please notice that this fix also changes behavior of RunTest.gmk when 
>>>>> conflicting combination of options is used. JCOV_IMAGE_DIR  is now 
>>>>> expected to be set when TEST_OPTS_JCOV is true.
>>>>> 
>>>>> Shura
>>>>> 
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8217761
>>>>> Webrev: http://cr.openjdk.java.net/~shurailine/8217761/webrev.02/
> 

Reply via email to