[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-compiler-dev
Subject: Re: Review Request : JDK-8031670: remove unneeded -source option from javadoc tests
From: Joe Darcy <joe.darcy () oracle ! com>
Date: 2014-02-24 17:12:46
Message-ID: 530B7D8E.5030104 () oracle ! com
[Download RAW message or body]
FWIW, I've already filed several bugs against JDK 10 for exactly that
purpose, recording some of the details of the version update from 8 -> 9
so we don't have to rediscover all those details going from 9 -> 10.
-Joe
On 02/24/2014 08:04 AM, Neil Toda wrote:
>
> Thanks Vicente. I like that idea of adding to JBS flagging future
> required changes.
>
> -neil
>
> On 2/24/2014 5:51 AM, Vicente-Arturo Romero-Zaldivar wrote:
>> Hi Neil,
>>
>> Some comments below.
>>
>> On 21/02/14 21:28, Neil Toda wrote:
>>>
>>>
>>> Hi Vicente.. some additional thoughts/comments below...
>>>
>>> On 2/21/2014 12:53 PM, Vicente-Arturo Romero-Zaldivar wrote:
>>>> Hi Neil,
>>>>
>>>> On 21/02/14 01:25, Neil Toda wrote:
>>>>>
>>>>> There is another test that does a negative check on a feature,
>>>>> testLambdaFeature.
>>>>> However, it too will stop working when support for 1.7 is removed
>>>>> from the JDK.
>>>>> Granted that will be several years from now, but by then we'll all
>>>>> forget and just
>>>>> remove the negative test.
>>>>
>>>> I hope that we won't ever remove negative tests just because they fail.
>>>>
>>>
>>> This test has two parts. The first checks to see that lambda is
>>> recognized.
>>> The second part checks that if the version supplied via -source does
>>> not support
>>> lambda, the case is correctly handled.
>>>
>>> When 1.7 is no longer supported in the JDK, then specifying "-source
>>> 1.7" will not be
>>> valid. At that point, all supported versions will support lambda.
>>
>> I would create a task in JBS, something like, update source option in
>> tests where you can add the comments of what needs to be done and
>> what tests will need to be modified. Then add a comment in all
>> affected tests referring to this JBS entry, see for example:
>> https://bugs.openjdk.java.net/browse/JDK-8006694 and the associated
>> changeset. This way we won't forget all your findings.
>>
>>
>>
>>>
>>> So I mean that in that case, the second part of the test is no
>>> longer needed nor valid.
>>>
>>>
>>>>>
>>>>> I think I've made a mistake taking out the ./sourceOption test.
>>>>> It is better that this
>>>>> test exist and be specifically designed to make sure -source is
>>>>> working.
>>>>
>>>> Agree.
>>>
>>> So, below when you say you'd like to see the proposed change, you
>>> are referring to statement
>>> about leaving ./sourceOption in? I'm just suggesting that my note
>>> below be added to ./sourceOption
>>> in the form of comment giving guidance for future releases, when
>>> 1.6, then 1.7 is no longer supported.
>>
>> OK, I agree.
>>
>>>
>>> -neil
>>
>> - Vicente
>>
>>>
>>>>
>>>>>
>>>>> With that in mind, I'll not remove it, and add to it so that we
>>>>> have a list of features
>>>>> in each release that will fail in the next release. Something like:
>>>>>
>>>>> Release Feature
>>>>> ======= ===================
>>>>> 1.6 List<String> list = ArrayList<String>();
>>>>> 1.7 List<String> list = ArrayList<>();
>>>>> 1.8 Lambda or default methods
>>>>> 1.9 TBD
>>>>>
>>>>> So for JDK9, the test will specify:
>>>>> -source 1.6
>>>>> A.java will contain : List<String> list = ArrayList<>();
>>>>>
>>>>> and the test will look for failure.
>>>>>
>>>>> In this way, the intent will by conveyed in the test.
>>>>>
>>>>> Does this all sound okay?
>>>>
>>>> I would prefer to see the proposed change, I'm not sure I fully
>>>> understand what you want to do here.
>>>>
>>>> Thanks,
>>>> Vicente
>>>>>
>>>>> Thanks Joe
>>>>>
>>>>> -neil
>>>>>
>>>>>
>>>>> On 2/19/2014 3:42 PM, Joseph Darcy wrote:
>>>>>> Hi Neil,
>>>>>>
>>>>>> On the removal of
>>>>>>
>>>>>> test/tools/javadoc/sourceOption/SourceOption.java
>>>>>>
>>>>>> after your patch is there some remaining javadoc test which uses
>>>>>> the -source option? We probably want to keep one test which uses
>>>>>> to the option to make sure it is supported. If there is no such
>>>>>> remaining test, SourceOption.java could be resurrected and make
>>>>>> to check that a JDK 8 feature is rejected under source 7, etc.
>>>>>>
>>>>>> Otherwise, the removal of the other -source uses looks fine.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Joe
>>>>>>
>>>>>> On 2/19/2014 7:37 AM, Vicente-Arturo Romero-Zaldivar wrote:
>>>>>>> Hi Neil,
>>>>>>>
>>>>>>> there is a typo at test/tools/javadoc/6964914/JavacWarning.java:
>>>>>>>
>>>>>>> "It *amy* be deprecated in JDK8"
>>>>>>>
>>>>>>> at test/com/sun/javadoc/testLambdaFeature/TestLambdaFeature.java:
>>>>>>>
>>>>>>> I would add the note as an independent comment after the
>>>>>>>
>>>>>>> * @test
>>>>>>> * @bug 8004893 8022738
>>>>>>> section not as part of it.
>>>>>>>
>>>>>>> Vicente
>>>>>>>
>>>>>>> On 18/02/14 21:10, Neil Toda wrote:
>>>>>>>>
>>>>>>>> A small set of javadoc tests contain the -source parameter. In
>>>>>>>> most cases, the parameter is
>>>>>>>> not required for the test. In several cases, the -source
>>>>>>>> parameter is being explicitly tested,
>>>>>>>> but relies on a JDK version that will be removed from JDK9.
>>>>>>>>
>>>>>>>> This changeset removes unnecessary -source specification when
>>>>>>>> not needed, or changes the
>>>>>>>> test/source-version requested to one that will work in JDK9.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~darcy/neiltoda/8031670.0/
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> -neil
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">FWIW, I've already filed several bugs
against JDK 10 for exactly that purpose, recording some of the
details of the version update from 8 -> 9 so we don't have to
rediscover all those details going from 9 -> 10.<br>
<br>
-Joe<br>
<br>
On 02/24/2014 08:04 AM, Neil Toda wrote:<br>
</div>
<blockquote cite="mid:530B6D9C.5020406@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<br>
Thanks Vicente. I like that idea of adding to JBS flagging future
required changes.<br>
<br>
-neil<br>
<br>
<div class="moz-cite-prefix">On 2/24/2014 5:51 AM, Vicente-Arturo
Romero-Zaldivar wrote:<br>
</div>
<blockquote cite="mid:530B4E61.9000404@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Neil,<br>
<br>
Some comments below.<br>
<br>
On 21/02/14 21:28, Neil Toda wrote:<br>
</div>
<blockquote cite="mid:5307C4FC.5050605@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<br>
<div class="moz-cite-prefix"><br>
Hi Vicente.. some additional thoughts/comments below...<br>
<br>
On 2/21/2014 12:53 PM, Vicente-Arturo Romero-Zaldivar wrote:<br>
</div>
<blockquote cite="mid:5307BCC1.2050109@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Neil,<br>
<br>
On 21/02/14 01:25, Neil Toda wrote:<br>
</div>
<blockquote cite="mid:5306AB1C.6050309@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<br>
There is another test that does a negative check on a
feature, testLambdaFeature.<br>
However, it too will stop working when support for 1.7 is
removed from the JDK.<br>
Granted that will be several years from now, but by then
we'll all forget and just<br>
remove the negative test.<br>
</blockquote>
<br>
I hope that we won't ever remove negative tests just because
they fail.<br>
<br>
</blockquote>
<br>
This test has two parts. The first checks to see that lambda
is recognized.<br>
The second part checks that if the version supplied via
-source does not support<br>
lambda, the case is correctly handled.<br>
<br>
When 1.7 is no longer supported in the JDK, then specifying
"-source 1.7" will not be<br>
valid. At that point, all supported versions will support
lambda.<br>
</blockquote>
<br>
I would create a task in JBS, something like, update source
option in tests where you can add the comments of what needs to
be done and what tests will need to be modified. Then add a
comment in all affected tests referring to this JBS entry, see
for example: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8006694">https://bugs.openjdk.java.net/browse/JDK-8006694</a>
and the associated changeset. This way we won't forget all your
findings.<br>
<br>
<br>
<br>
<blockquote cite="mid:5307C4FC.5050605@oracle.com" type="cite">
<br>
So I mean that in that case, the second part of the test is no
longer needed nor valid.<br>
<br>
<br>
<blockquote cite="mid:5307BCC1.2050109@oracle.com" type="cite">
<blockquote cite="mid:5306AB1C.6050309@oracle.com"
type="cite"> <br>
I think I've made a mistake taking out the ./sourceOption
test. It is better that this<br>
test exist and be specifically designed to make sure
-source is working.<br>
</blockquote>
<br>
Agree.<br>
</blockquote>
<br>
So, below when you say you'd like to see the proposed change,
you are referring to statement<br>
about leaving ./sourceOption in? I'm just suggesting that my
note below be added to ./sourceOption<br>
in the form of comment giving guidance for future releases,
when 1.6, then 1.7 is no longer supported.<br>
</blockquote>
<br>
OK, I agree.<br>
<br>
<blockquote cite="mid:5307C4FC.5050605@oracle.com" type="cite">
<br>
-neil<br>
</blockquote>
<br>
- Vicente<br>
<br>
<blockquote cite="mid:5307C4FC.5050605@oracle.com" type="cite">
<br>
<blockquote cite="mid:5307BCC1.2050109@oracle.com" type="cite">
<br>
<blockquote cite="mid:5306AB1C.6050309@oracle.com"
type="cite"> <br>
With that in mind, I'll not remove it, and add to it so
that we have a list of features<br>
in each release that will fail in the next release.
Something like:<br>
<br>
<tt>Release Feature</tt><tt><br>
</tt><tt>======= ===================<br>
1.6 List<String> list =
ArrayList<String>();<br>
1.7 List<String> list =
ArrayList<>();<br>
1.8 Lambda or default methods<br>
1.9 TBD<br>
<br>
So for JDK9, the test will specify:<br>
-source 1.6<br>
A.java will contain : List<String> list =
ArrayList<>();<br>
<br>
and the test will look for failure.<br>
<br>
In this way, the intent will by conveyed in the test.<br>
<br>
Does this all sound okay?<br>
</tt></blockquote>
<br>
I would prefer to see the proposed change, I'm not sure I
fully understand what you want to do here.<br>
<br>
Thanks,<br>
Vicente<br>
<blockquote cite="mid:5306AB1C.6050309@oracle.com"
type="cite"><tt> <br>
Thanks Joe<br>
<br>
-neil<br>
<br>
</tt><br>
<div class="moz-cite-prefix">On 2/19/2014 3:42 PM, Joseph
Darcy wrote:<br>
</div>
<blockquote cite="mid:53054165.8080502@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
Hi Neil,<br>
<br>
On the removal of<br>
<br>
test/tools/javadoc/sourceOption/SourceOption.java<br>
<br>
after your patch is there some remaining javadoc test
which uses the -source option? We probably want to keep
one test which uses to the option to make sure it is
supported. If there is no such remaining test,
SourceOption.java could be resurrected and make to check
that a JDK 8 feature is rejected under source 7, etc.<br>
<br>
Otherwise, the removal of the other -source uses looks
fine.<br>
<br>
Thanks,<br>
<br>
-Joe<br>
<br>
<div class="moz-cite-prefix">On 2/19/2014 7:37 AM,
Vicente-Arturo Romero-Zaldivar wrote:<br>
</div>
<blockquote cite="mid:5304CFB8.3000703@oracle.com"
type="cite">
<meta content="text/html; charset=UTF-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi Neil,<br>
<br>
there is a typo at
test/tools/javadoc/6964914/JavacWarning.java:<br>
<br>
"It <b>amy</b> be deprecated in JDK8"<br>
<br>
at
test/com/sun/javadoc/testLambdaFeature/TestLambdaFeature.java:<br>
<br>
I would add the note as an independent comment after
the<br>
<br>
<pre>* @test
* @bug 8004893 8022738</pre>
section not as part of it.<br>
<br>
Vicente<br>
<br>
On 18/02/14 21:10, Neil Toda wrote:<br>
</div>
<blockquote cite="mid:5303CC29.8090909@oracle.com"
type="cite"> <br>
A small set of javadoc tests contain the -source
parameter. In most cases, the parameter is <br>
not required for the test. In several cases, the
-source parameter is being explicitly tested, <br>
but relies on a JDK version that will be removed
from JDK9. <br>
<br>
This changeset removes unnecessary -source
specification when not needed, or changes the <br>
test/source-version requested to one that will work
in JDK9. <br>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="http://cr.openjdk.java.net/%7Edarcy/neiltoda/8031670.0/">http://cr.openjdk.java.net/~darcy/neiltoda/8031670.0/</a>
<br>
<br>
Thanks <br>
<br>
-neil <br>
<br>
<br>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic