> On Jan 30, 2019, at 8:53 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
>
>
> On 2019-01-30 06:48, Magnus Ihse Bursie 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.
>>
> Ah I see, but the previously existing conditional only checked if the
> variable JCOV_IMAGE_DIR was set, not if the directory it pointed to existed,
> so it wasn't any better. We set the variable JCOV_IMAGE_DIR unconditionally
> in spec.gmk, so that check would always be true anyway.
>
> What you are asking for is adding a new check that JCOV_HOME is set (this is
> a consequence of setting --with-jcov) and that JCOV_IMAGE_DIR actually points
> to something valid (this is the result of building jcov-image). It would also
> be nice if the jcov-image target failed with a reasonable error if JCOV_HOME
> wasn't set. I agree that these checks would be useful.
I will fix that, unless somebody else really wants to take it.
Shura.
>
> /Erik
>
>> /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/
>>
