Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Ao Qi
Hi,

The original email is waiting for moderator approval. Subscribed to
jigsaw-dev and resent the RFR (cc'ed core-libs-dev).

Thanks,
Ao Qi

On Wed, Apr 15, 2020 at 10:11 PM Ao Qi <[hidden email]> wrote:

>
> Hi all,
>
> test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java was changed
> to an empty file by JDK-8162538, but not removed. I think we should
> remve this file.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8242846
> webrev: http://cr.openjdk.java.net/~aoqi/8242846/webrev.00/
>
> Thanks,
> Ao Qi

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Alan Bateman

Sundar - do you recognize this? I assume truncating this test to 0 bytes
was a mistake but I can't tell if the test should be brought back or not
(to ensure that`jlink --order-resources` is tested).

-Alan

On 17/04/2020 11:34, Ao Qi wrote:

> Hi,
>
> The original email is waiting for moderator approval. Subscribed to
> jigsaw-dev and resent the RFR (cc'ed core-libs-dev).
>
> Thanks,
> Ao Qi
>
> On Wed, Apr 15, 2020 at 10:11 PM Ao Qi <[hidden email]> wrote:
>> Hi all,
>>
>> test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java was changed
>> to an empty file by JDK-8162538, but not removed. I think we should
>> remve this file.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242846
>> webrev: http://cr.openjdk.java.net/~aoqi/8242846/webrev.00/
>>
>> Thanks,
>> Ao Qi

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

sundararajan.athijegannathan
Hi Alan,

I don't remember it now. Likely a mistake. The changeset

http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42

has that file. Perhaps it may be useful to restore & check if the test
passes.

Thanks,

-Sundar

On 18/04/20 3:32 pm, Alan Bateman wrote:

>
> Sundar - do you recognize this? I assume truncating this test to 0
> bytes was a mistake but I can't tell if the test should be brought
> back or not (to ensure that`jlink --order-resources` is tested).
>
> -Alan
>
> On 17/04/2020 11:34, Ao Qi wrote:
>> Hi,
>>
>> The original email is waiting for moderator approval. Subscribed to
>> jigsaw-dev and resent the RFR (cc'ed core-libs-dev).
>>
>> Thanks,
>> Ao Qi
>>
>> On Wed, Apr 15, 2020 at 10:11 PM Ao Qi <[hidden email]> wrote:
>>> Hi all,
>>>
>>> test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java was changed
>>> to an empty file by JDK-8162538, but not removed. I think we should
>>> remve this file.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242846
>>> webrev: http://cr.openjdk.java.net/~aoqi/8242846/webrev.00/
>>>
>>> Thanks,
>>> Ao Qi
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Alan Bateman
On 20/04/2020 11:32, [hidden email] wrote:
> Hi Alan,
>
> I don't remember it now. Likely a mistake. The changeset
>
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>
> has that file. Perhaps it may be useful to restore & check if the test
> passes.
Yes. I think it was the only test for this jlink option so let's bring
it back or replace it.

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Ao Qi
In reply to this post by sundararajan.athijegannathan
Hi Alan and Sundar,


Thanks for the advice. I will take a look at if the test should be
brought back.


Cheers,

Ao Qi


On 2020/4/20 下午6:32, [hidden email] wrote:

> Hi Alan,
>
> I don't remember it now. Likely a mistake. The changeset
>
> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>
> has that file. Perhaps it may be useful to restore & check if the test
> passes.
>
> Thanks,
>
> -Sundar
>
> On 18/04/20 3:32 pm, Alan Bateman wrote:
>>
>> Sundar - do you recognize this? I assume truncating this test to 0
>> bytes was a mistake but I can't tell if the test should be brought
>> back or not (to ensure that`jlink --order-resources` is tested).
>>
>> -Alan
>>
>> On 17/04/2020 11:34, Ao Qi wrote:
>>> Hi,
>>>
>>> The original email is waiting for moderator approval. Subscribed to
>>> jigsaw-dev and resent the RFR (cc'ed core-libs-dev).
>>>
>>> Thanks,
>>> Ao Qi
>>>
>>> On Wed, Apr 15, 2020 at 10:11 PM Ao Qi <[hidden email]> wrote:
>>>> Hi all,
>>>>
>>>> test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java was changed
>>>> to an empty file by JDK-8162538, but not removed. I think we should
>>>> remve this file.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8242846
>>>> webrev: http://cr.openjdk.java.net/~aoqi/8242846/webrev.00/
>>>>
>>>> Thanks,
>>>> Ao Qi
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Ao Qi
In reply to this post by Alan Bateman

On 2020/4/20 下午9:27, Alan Bateman wrote:

> On 20/04/2020 11:32, [hidden email] wrote:
>> Hi Alan,
>>
>> I don't remember it now. Likely a mistake. The changeset
>>
>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>>
>> has that file. Perhaps it may be useful to restore & check if the
>> test passes.
> Yes. I think it was the only test for this jlink option so let's bring
> it back or replace it.
>

I think the test should be brought back. I restored the test and it
didn't pass. Here is the diff against the original test[1]:
http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev is
http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not familiar
with this field, so I am not sure I fix this rightly.


Cheers,
Ao Qi

[1]
http://hg.openjdk.java.net/jdk9/jdk9/jdk/raw-file/decf1acca62d/test/tools/jlink/plugins/OrderResourcesPluginTest.java


> -Alan.

Reply | Threaded
Open this post in threaded view
|

Re: RFR (T): 8242846: removed an empty file test/jdk/tools/jlink/plugins/OrderResourcesPluginTest.java

Alan Bateman
On 21/04/2020 04:58, Ao Qi wrote:

>
> On 2020/4/20 下午9:27, Alan Bateman wrote:
>> On 20/04/2020 11:32, [hidden email] wrote:
>>> Hi Alan,
>>>
>>> I don't remember it now. Likely a mistake. The changeset
>>>
>>> http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/a066fe7b1b42
>>>
>>> has that file. Perhaps it may be useful to restore & check if the
>>> test passes.
>> Yes. I think it was the only test for this jlink option so let's
>> bring it back or replace it.
>>
>
> I think the test should be brought back. I restored the test and it
> didn't pass. Here is the diff against the original test[1]:
> http://cr.openjdk.java.net/~aoqi/8242846/diff.01, and the new webrev
> is http://cr.openjdk.java.net/~aoqi/8242846/webrev.01/ . I'm not
> familiar with this field, so I am not sure I fix this rightly.
Thanks, I think this looks okay.

-Alan.