Integrated: 8183374: Refactor java/lang/Runtime shell tests to java

[Fernando Guallini]

On Mon, 19 Apr 2021 15:07:16 GMT, Fernando Guallini  
wrote:

> Refactor the following shell tests to java:
> test/jdk/java/lang/RuntimeTests/shutdown/ShutdownHooks.sh
> test/jdk/java/lang/Runtime/exec/SetCwd.java
> 
> In addition, the test SetCwd was running itself in separate java subprocesses 
> in order to exercise Runtime.exec. It was creating a folder structure with 
> multiple test class copies to distinguish between main and child processes to 
> prevent an infinite recursion. That logic is simplified now, tests follow the 
> testng annotations flow whereas the subprocesses entry point is a nested 
> class main

This pull request has now been integrated.

Changeset: ec383abc
Author:Fernando Guallini 
Committer: Sean Coffey 
URL:   
https://git.openjdk.java.net/jdk/commit/ec383abc1d2e609cc6af94a526e11c407d7e91ff
Stats: 232 lines in 4 files changed: 67 ins; 121 del; 44 mod

8183374: Refactor java/lang/Runtime shell tests to java

Reviewed-by: coffeys

-

PR: https://git.openjdk.java.net/jdk/pull/3572

Re: RFR: 8183374: Refactor java/lang/Runtime shell tests to java

[Fernando Guallini]

On Mon, 19 Apr 2021 15:07:16 GMT, Fernando Guallini  
wrote:

> Refactor the following shell tests to java:
> test/jdk/java/lang/RuntimeTests/shutdown/ShutdownHooks.sh
> test/jdk/java/lang/Runtime/exec/SetCwd.java
> 
> In addition, the test SetCwd was running itself in separate java subprocesses 
> in order to exercise Runtime.exec. It was creating a folder structure with 
> multiple test class copies to distinguish between main and child processes to 
> prevent an infinite recursion. That logic is simplified now, tests follow the 
> testng annotations flow whereas the subprocesses entry point is a nested 
> class main

Please, have a look at this PR, it would also need sponsoring, thanks

-

PR: https://git.openjdk.java.net/jdk/pull/3572

RFR: 8183374: Refactor java/lang/Runtime shell tests to java

[Fernando Guallini]

Refactor the following shell tests to java:
test/jdk/java/lang/RuntimeTests/shutdown/ShutdownHooks.sh
test/jdk/java/lang/Runtime/exec/SetCwd.java

In addition, the test SetCwd was running itself in separate java subprocesses 
in order to exercise Runtime.exec. It was creating a folder structure with 
multiple test class copies to distinguish between main and child processes to 
prevent an infinite recursion. That logic is simplified now, tests follow the 
testng annotations flow whereas the subprocesses entry point is a nested class 
main

-

Commit messages:
 - Merge branch 'master' of github.com:openjdk/jdk into 8183374
 - refactor ShutdownHooks and SetCwd

Changes: https://git.openjdk.java.net/jdk/pull/3572/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3572=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8183374
  Stats: 232 lines in 4 files changed: 67 ins; 121 del; 44 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3572.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3572/head:pull/3572

PR: https://git.openjdk.java.net/jdk/pull/3572

Re: RFR: 8249694 - [TestBug] java/lang/StringBuffer/HugeCapacity.java and j/l/StringBuilder/HugeCapacity.java tests shouldn't be @ignore-d

[Fernando Guallini]

Hi Sean,
Right, it also applies for these tests, changes:
--- a/test/jdk/java/lang/StringBuffer/HugeCapacity.java
+++ b/test/jdk/java/lang/StringBuffer/HugeCapacity.java
- * @requires os.maxMemory >= 6G
+ * @requires (sun.arch.data.model == "64" & os.maxMemory >= 6G)
--- a/test/jdk/java/lang/StringBuilder/HugeCapacity.java
+++ b/test/jdk/java/lang/StringBuilder/HugeCapacity.java
- * @requires os.maxMemory >= 6G
+ * @requires (sun.arch.data.model == "64" & os.maxMemory >= 6G)

Regards,
Fernando

> On 1 Sep 2020, at 17:25, Seán Coffey  wrote:
> 
> Wouldn't you require the sun.arch.data.model == "64" jtreg config in these 
> tests also ?
> 
> regards,
> Sean.
> 
> On 28/08/2020 19:13, Fernando Guallini wrote:
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> Hi,
>> 
>> May I please get reviews and a sponsor for this trivial change:
>> 
>> webrev: http://cr.openjdk.java.net/~fguallini/8249694/webrev.00/
>> Testbug: https://bugs.openjdk.java.net/browse/JDK-8249694
>> 
>> Tests do not need to have ‘@ignore' because with @requires os.maxMemory is 
>> enough to ensure they will not be executed if memory requirements are not 
>> satisfied. They run in Mach5 with no issues.
>> 
>> Thanks
>> 
>> -Fernando

RFR: 8249694 - [TestBug] java/lang/StringBuffer/HugeCapacity.java and j/l/StringBuilder/HugeCapacity.java tests shouldn't be @ignore-d

[Fernando Guallini]

Hi, 

May I please get reviews and a sponsor for this trivial change: 

webrev: http://cr.openjdk.java.net/~fguallini/8249694/webrev.00/ 
Testbug: https://bugs.openjdk.java.net/browse/JDK-8249694 

Tests do not need to have ‘@ignore' because with @requires os.maxMemory is 
enough to ensure they will not be executed if memory requirements are not 
satisfied. They run in Mach5 with no issues. 

Thanks 

-Fernando

Re: RFR: JDK-8249699: java/io/ByteArrayOutputStream/MaxCapacity.java should use @requires instead of @ignore

[Fernando Guallini]

Thanks Sean, updated webrev here: 
http://cr.openjdk.java.net/~fguallini/8249699/webrev.01/

Regards,
Fernando
- Original Message -
From: sean.cof...@oracle.com
To: fernando.guall...@oracle.com, core-libs-dev@openjdk.java.net
Sent: Wednesday, 26 August, 2020 7:39:25 PM GMT +00:00 GMT Britain, Ireland, 
Portugal
Subject: Re: RFR: JDK-8249699: java/io/ByteArrayOutputStream/MaxCapacity.java 
should use @requires instead of @ignore

test/jdk/java/util/Base64/TestEncodingDecodingLength.java is an example 
of another test using -Xmx8g. Do you want to push the os.maxMemory 
requirement up to 10g perhaps ? It might avoid border line resource 
failures. Also I think it might need a "sun.arch.data.model == "64" " 
requirement :

@requires (sun.arch.data.model == "64" & os.maxMemory >= 10g)

regards,
Sean.

On 26/08/2020 18:17, Fernando Guallini wrote:
> Hi,
>
> Could I please get reviews and a sponsor for:
>
> webrev: http://cr.openjdk.java.net/~fguallini/8249699/webrev.00/ 
> <http://cr.openjdk.java.net/~fguallini/8249699/webrev.00/>
> Testbug: https://bugs.openjdk.java.net/browse/JDK-8249699 
> <https://bugs.openjdk.java.net/browse/JDK-8249699>
>
> The test was ignored due to ‘huge memory requirements’, but with the jtreg 
> current version, tests can be only run when system requirements are satisfied 
> as opposed to being always excluded. It runs and passes in Mach5
>
>
> Thanks
>
> -Fernando

RFR: JDK-8249699: java/io/ByteArrayOutputStream/MaxCapacity.java should use @requires instead of @ignore

[Fernando Guallini]

Hi,

Could I please get reviews and a sponsor for:

webrev: http://cr.openjdk.java.net/~fguallini/8249699/webrev.00/ 

Testbug: https://bugs.openjdk.java.net/browse/JDK-8249699 


The test was ignored due to ‘huge memory requirements’, but with the jtreg 
current version, tests can be only run when system requirements are satisfied 
as opposed to being always excluded. It runs and passes in Mach5


Thanks

-Fernando

Re: RFR: JDK-8222241 - Example in ServiceLoader API docs should have one provides directive

[Fernando Guallini]

Sure, added indentation here: 
http://cr.openjdk.java.net/~pconcannon/fguallin/841/webrevs/webrev.01/ 
<http://cr.openjdk.java.net/~pconcannon/fguallin/841/webrevs/webrev.01/>

This is how the html is rendered:






Thanks



> On 29 May 2020, at 19:00, Alan Bateman  wrote:
> 
> On 29/05/2020 18:12, Fernando Guallini wrote:
>> Hi,
>> in the ServiceLoader API docs, the given example specifying the service 
>> providers for a particular service should be comma-separated instead of 
>> using two separate ‘provides’ directives, that would result in a compilation 
>> error.
>> 
>> Web rev: 
>> http://cr.openjdk.java.net/~pconcannon/fguallin/841/webrevs/webrev.00/ 
>> <http://cr.openjdk.java.net/~pconcannon/fguallin/841/webrevs/webrev.00/>
>> 
>> Bug: https://bugs.openjdk.java.net/browse/JDK-841 
>> <https://bugs.openjdk.java.net/browse/JDK-841>
>> 
> Yes, it should be one `provides` directive. Can you intend the second class 
> name to make the code fragment a bit more readable in the generated javadoc?
> 
> -Alan

RFR: JDK-8222241 - Example in ServiceLoader API docs should have one provides directive

[Fernando Guallini]

Hi, 
in the ServiceLoader API docs, the given example specifying the service 
providers for a particular service should be comma-separated instead of using 
two separate ‘provides’ directives, that would result in a compilation error.

Web rev: 
http://cr.openjdk.java.net/~pconcannon/fguallin/841/webrevs/webrev.00/ 
 

Bug: https://bugs.openjdk.java.net/browse/JDK-841 


Thanks,
Fernando

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

[Fernando Guallini]

Thanks for your comments.
Below you can find a new web rev version that includes a test description in a 
comment:
http://cr.openjdk.java.net/~fyuan/fernando/8209774/webrev.01/ 
<http://cr.openjdk.java.net/~fyuan/fernando/8209774/webrev.01/>

I will also create a JBS ticket to revisit this test and remove it if obsolete, 
if that is fine with you.

-Fernando

> On 11 May 2020, at 17:27, Joe Wang  wrote:
> 
> 
> 
> On 5/11/2020 2:32 AM, Alan Bateman wrote:
>> On 08/05/2020 18:19, Fernando Guallini wrote:
>>> Hi Daniel and Alan,
>>> @compile/module=java.xml was my first try, but for the nature of this test, 
>>> it didn't work. The reason is that the original shell test does the 
>>> following:
>>> - Compiles it’s own version of Node and Document interfaces
>>> - Compiles DocumentImpl patching java.xml with those 2 interfaces
>>> - Executes the AbstractMethodErrorTest patching the java.xml module *only 
>>> with DocumentImpl* patch(not including Document and Node)
>>> 
>>> It does that by keeping the patches output in different folders. That’s 
>>> important otherwise AbstractMethodErrorTest doesn’t compile, because it 
>>> references some methods not declared in its custom interfaces, and it seems 
>>> to be coded this way to reproduce the original bug. That is also the reason 
>>> why I added the *@clean* command to remove Document and Node *before* 
>>> running the test.
>>> 
>>> But when using *@compile/module=java.xml*, under the hood, JTREG generates 
>>> a folder named “patches/java.xml/..” including all the compiled classes 
>>> under it. Unfortunately, the temporary interfaces can’t be removed because 
>>> *@clean* does not know how to resolve the path "/patches/java.xml/..”.
>>> 
>>> To sum up, this test creates a temporary java.xml patch that needs to be 
>>> ignored after compiling *DocumentImpl. *I am using —patch-module because 
>>> it’s more flexible than @compile/module
>>> *
>>> *
>>> Hope I explained myself!
>> This may be a question for Joe Wang but I'm curious if this need to 
>> override/upgrade W3C DOM API classes dates back to when it was an endorsed 
>> standard that could be overridden with the endorsed standards override 
>> mechanism. The @bug on the test suggests otherwise but I'm curious if it 
>> make any sense with JDK 9+. This doesn't impact the good work to replace the 
>> script of course, I'm just curious how separately compilation issue arises 
>> here.
> 
> I agree with you, Alan, that this test is obsolete. I just haven't thought of 
> getting rid of it. But you're right, it can be removed instead.
> 
> -Joe
> 
>> 
>> -Alan
>> 
> 

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

[Fernando Guallini]

Sure, I will add some of the explanation as a comment. Thank you Daniel!

> On 8 May 2020, at 18:39, Daniel Fuchs  wrote:
> 
> Hi Fernando,
> 
> Ah - I see the keypoint is the:
>  39  * @clean org.w3c.dom.*
> OK, I missed that. Then I agree - ignore my previous comments.
> 
> Maybe the explanations below should be added to the test in some
> comment to help future maintainers (and reviewers).
> 
> best regards,
> 
> -- daniel
> 
> 
> On 08/05/2020 18:19, Fernando Guallini wrote:
>> Hi Daniel and Alan,
>> @compile/module=java.xml was my first try, but for the nature of this test, 
>> it didn't work. The reason is that the original shell test does the 
>> following:
>> - Compiles it’s own version of Node and Document interfaces
>> - Compiles DocumentImpl patching java.xml with those 2 interfaces
>> - Executes the AbstractMethodErrorTest patching the java.xml module *only 
>> with DocumentImpl* patch(not including Document and Node)
>> It does that by keeping the patches output in different folders. That’s 
>> important otherwise AbstractMethodErrorTest doesn’t compile, because it 
>> references some methods not declared in its custom interfaces, and it seems 
>> to be coded this way to reproduce the original bug. That is also the reason 
>> why I added the *@clean* command to remove Document and Node *before* 
>> running the test.
>> But when using *@compile/module=java.xml*, under the hood, JTREG generates a 
>> folder named “patches/java.xml/..” including all the compiled classes under 
>> it. Unfortunately, the temporary interfaces can’t be removed because 
>> *@clean* does not know how to resolve the path "/patches/java.xml/..”.
>> To sum up, this test creates a temporary java.xml patch that needs to be 
>> ignored after compiling *DocumentImpl. *I am using —patch-module because 
>> it’s more flexible than @compile/module
>> *
>> *
>> Hope I explained myself!
>>> On 8 May 2020, at 15:49, Daniel Fuchs >> <mailto:daniel.fu...@oracle.com>> wrote:
>>> 
>>> On 08/05/2020 15:40, Alan Bateman wrote:
>>>>>   31 /*
>>>>>   32  * @test
>>>>>   33  * @bug 8035437
>>>>>   34  * @summary Tests that java.lang.AbstractMethodError is not thrown 
>>>>> when
>>>>>   35  * serializing improper version of DocumentImpl class.
>>>>>   36  * @library /test/lib
>>>>>   * @modules javax.xml/org.w3c.dom
>>>>>   *  javax.xml/com.sun.org.apache.xerces.internal.dom
>>>>>   40  * @run main/othervm --patch-module java.xml=${test.class.path} 
>>>>> AbstractMethodErrorTest
>>>>>   41  */
>>>>> 
>>>>> (not 100% sure the @modules is even needed)
>>>> I wouldn't expect to need --patch-module here. Instead maybe it could be 
>>>> changed to use @compile/module=java.xml ... and jtreg should compile and 
>>>> run the overrides "as if" they are in the java.xml module. There are a 
>>>> couple of examples of this in the test suite that might help get this 
>>>> going. No need for javax.xml/org.w3c.dom as that package is already 
>>>> exported.
>>> 
>>> Right. Copy paste error. The --patch-module shouldn't be needed
>>> anywhere. Good point about @compile - the main class
>>> AbstractMethodErrorTest is not in the patched module, so
>>> the patched classes may not get compiled if you don't force
>>> their compilation.
>>> 
>>> Thanks for the correction Alan!
>>> 
>>> -- daniel
> 

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

[Fernando Guallini]

Hi Daniel and Alan,
@compile/module=java.xml was my first try, but for the nature of this test, it 
didn't work. The reason is that the original shell test does the following:
- Compiles it’s own version of Node and Document interfaces  
- Compiles DocumentImpl patching java.xml with those 2 interfaces
- Executes the AbstractMethodErrorTest patching the java.xml module only with 
DocumentImpl patch(not including Document and Node)

It does that by keeping the patches output in different folders. That’s 
important otherwise AbstractMethodErrorTest doesn’t compile, because it 
references some methods not declared in its custom interfaces, and it seems to 
be coded this way to reproduce the original bug. That is also the reason why I 
added the @clean command to remove Document and Node before running the test.

But when using @compile/module=java.xml, under the hood, JTREG generates a 
folder named “patches/java.xml/..” including all the compiled classes under it. 
Unfortunately, the temporary interfaces can’t be removed because @clean does 
not know how to resolve the path "/patches/java.xml/..”. 

To sum up, this test creates a temporary java.xml patch that needs to be 
ignored after compiling DocumentImpl. I am using —patch-module because it’s 
more flexible than @compile/module

Hope I explained myself!


> On 8 May 2020, at 15:49, Daniel Fuchs  wrote:
> 
> On 08/05/2020 15:40, Alan Bateman wrote:
>>>   31 /*
>>>   32  * @test
>>>   33  * @bug 8035437
>>>   34  * @summary Tests that java.lang.AbstractMethodError is not thrown when
>>>   35  * serializing improper version of DocumentImpl class.
>>>   36  * @library /test/lib
>>>   * @modules javax.xml/org.w3c.dom
>>>   *  javax.xml/com.sun.org.apache.xerces.internal.dom
>>>   40  * @run main/othervm --patch-module java.xml=${test.class.path} 
>>> AbstractMethodErrorTest
>>>   41  */
>>> 
>>> (not 100% sure the @modules is even needed)
>> I wouldn't expect to need --patch-module here. Instead maybe it could be 
>> changed to use @compile/module=java.xml ... and jtreg should compile and run 
>> the overrides "as if" they are in the java.xml module. There are a couple of 
>> examples of this in the test suite that might help get this going. No need 
>> for javax.xml/org.w3c.dom as that package is already exported.
> 
> Right. Copy paste error. The --patch-module shouldn't be needed
> anywhere. Good point about @compile - the main class
> AbstractMethodErrorTest is not in the patched module, so
> the patched classes may not get compiled if you don't force
> their compilation.
> 
> Thanks for the correction Alan!
> 
> -- daniel

RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

[Fernando Guallini]

Hi all,

Please, review the following change:

webrev: http://cr.openjdk.java.net/~fyuan/fernando/8209774/webrev.00/ 

Testbug: https://bugs.openjdk.java.net/browse/JDK-8209774 


Change details:
- Refactor shell test to java

That test was originally created to check XERCESJ-1007 bug fix, that 
java.lang.AbstractMethodError is not thrown when patching JDK boot class 
DocumentImpl with its own. 

Kind regards,
Fernando

RFR[8183266] - [TESTBUG]Add test to cover XPathEvaluationResult.XPathResultType.getQNameType method

[Fernando Guallini]

Hi all,

Please, review the following change:

webrev: http://cr.openjdk.java.net/~joehw/jdk15/8183266/webrev/
bug: https://bugs.openjdk.java.net/browse/JDK-8183266

Change details:
- Added test coverage for XPathEvaluationResult.XPathResultType.getQNameType 
method
- Added type check for the getQNameType flow restricting the Number class 
subtypes to satisfy the spec (Integer, Double and Long)
- Updated equalsClassType method to be null safe.

Kind regards,
Fernando