Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
Hi Brent, thank you for the review.
On 4/6/17 1:08 PM, Brent Christian wrote:
Hi, Naoto
On 4/5/17 2:14 PM, Naoto Sato wrote:
I revised the test case not to rely on shell script.
Yay! Hopefully this can also happen sometime for JDK 9+.
Sure. Will work on it.
http://cr.openjdk.java.net/~naoto/816/webrev.01/
Looks fine to me, Naoto. A few comments:
* I presume additional @bug values will be added as other fixes are
backported (e.g. 8054214).
Good point. Will add those bug ids on push.
* On
73 Path dst = Paths.get("testjava").toAbsolutePath();
This places "dst" within the scratch directory, then? (I thought there
was a jtreg system property for the scratch directory, but I can't find
it so I think I'm mis-remembering).
I agree with letting jtreg take care of cleaning up "scratch".
Yes, "dst" will be "scratch". Not aware of any system property to
designate the output directory, other than "-w" command option to
specify the JTwork dir.
If anything goes wrong with copying of the JDK (e.g. full disk),
hopefully it would be easy to diagnose, with an IOException with a full
stack trace.
* Have you confirmed (if it's practical to do so) that this test fails
when expected (detects a bug)?
Yes. Without those backports, the test will fail as expected.
Naoto
Thanks,
-Brent
On 3/30/17 2:10 PM, Naoto Sato wrote:
Hello,
Please review the changes to the following issue:
https://bugs.openjdk.java.net/browse/JDK-816
The proposed change is located at:
http://cr.openjdk.java.net/~naoto/816/webrev.00/
This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.
Naoto
Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
Hi, Naoto
On 4/5/17 2:14 PM, Naoto Sato wrote:
I revised the test case not to rely on shell script.
Yay! Hopefully this can also happen sometime for JDK 9+.
http://cr.openjdk.java.net/~naoto/816/webrev.01/
Looks fine to me, Naoto. A few comments:
* I presume additional @bug values will be added as other fixes are
backported (e.g. 8054214).
* On
73 Path dst = Paths.get("testjava").toAbsolutePath();
This places "dst" within the scratch directory, then? (I thought there
was a jtreg system property for the scratch directory, but I can't find
it so I think I'm mis-remembering).
I agree with letting jtreg take care of cleaning up "scratch".
If anything goes wrong with copying of the JDK (e.g. full disk),
hopefully it would be easy to diagnose, with an IOException with a full
stack trace.
* Have you confirmed (if it's practical to do so) that this test fails
when expected (detects a bug)?
Thanks,
-Brent
On 3/30/17 2:10 PM, Naoto Sato wrote:
Hello,
Please review the changes to the following issue:
https://bugs.openjdk.java.net/browse/JDK-816
The proposed change is located at:
http://cr.openjdk.java.net/~naoto/816/webrev.00/
This is for backporting fixes for JapaneseEra related issues (8054214,
8173423). The original fixes in JDK9 included a test case,
SupplementalJapaneseEraTest which is intended for the system property
testing. The above proposed fix intends to adapt that test case into
JDK8, where calendars.properties file is used instead of the system
property. The test is pretty much identical to JDK9's. The difference is
to deal with the calendars.properties file and removed some
non-applicable cases in JDK8.
Naoto
Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
Thanks for reviewing, Roger. On 4/6/17 8:39 AM, Roger Riggs wrote: Hi Naoto, Thanks for replacing the shell script with Java code. Given the size of the JDK, I'd suggest removing the copy at the end of the test unless you can rely on jtreg to remove it promptly. I was thinking about using @AfterTest to do it, but then realized that jtreg automatically cleans up the "scratch" directory after the test, where the test JDK is copied into. Here is the quote from jtreg's page [1]: --- jtreg will automatically clean up any files written in the scratch directory. In general, you should not clean these files up within the test because they may provide infomation useful to diagnose a test failure, should that be necessary. --- Naoto [1] http://openjdk.java.net/jtreg/writetests.html The rest looks fine, Roger On 4/5/2017 5:14 PM, Naoto Sato wrote: I revised the test case not to rely on shell script. Please review. http://cr.openjdk.java.net/~naoto/816/webrev.01/ Naoto On 3/30/17 2:10 PM, Naoto Sato wrote: Hello, Please review the changes to the following issue: https://bugs.openjdk.java.net/browse/JDK-816 The proposed change is located at: http://cr.openjdk.java.net/~naoto/816/webrev.00/ This is for backporting fixes for JapaneseEra related issues (8054214, 8173423). The original fixes in JDK9 included a test case, SupplementalJapaneseEraTest which is intended for the system property testing. The above proposed fix intends to adapt that test case into JDK8, where calendars.properties file is used instead of the system property. The test is pretty much identical to JDK9's. The difference is to deal with the calendars.properties file and removed some non-applicable cases in JDK8. Naoto
Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
Hi Naoto, Thanks for replacing the shell script with Java code. Given the size of the JDK, I'd suggest removing the copy at the end of the test unless you can rely on jtreg to remove it promptly. The rest looks fine, Roger On 4/5/2017 5:14 PM, Naoto Sato wrote: I revised the test case not to rely on shell script. Please review. http://cr.openjdk.java.net/~naoto/816/webrev.01/ Naoto On 3/30/17 2:10 PM, Naoto Sato wrote: Hello, Please review the changes to the following issue: https://bugs.openjdk.java.net/browse/JDK-816 The proposed change is located at: http://cr.openjdk.java.net/~naoto/816/webrev.00/ This is for backporting fixes for JapaneseEra related issues (8054214, 8173423). The original fixes in JDK9 included a test case, SupplementalJapaneseEraTest which is intended for the system property testing. The above proposed fix intends to adapt that test case into JDK8, where calendars.properties file is used instead of the system property. The test is pretty much identical to JDK9's. The difference is to deal with the calendars.properties file and removed some non-applicable cases in JDK8. Naoto
Re: [8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
I revised the test case not to rely on shell script. Please review. http://cr.openjdk.java.net/~naoto/816/webrev.01/ Naoto On 3/30/17 2:10 PM, Naoto Sato wrote: Hello, Please review the changes to the following issue: https://bugs.openjdk.java.net/browse/JDK-816 The proposed change is located at: http://cr.openjdk.java.net/~naoto/816/webrev.00/ This is for backporting fixes for JapaneseEra related issues (8054214, 8173423). The original fixes in JDK9 included a test case, SupplementalJapaneseEraTest which is intended for the system property testing. The above proposed fix intends to adapt that test case into JDK8, where calendars.properties file is used instead of the system property. The test is pretty much identical to JDK9's. The difference is to deal with the calendars.properties file and removed some non-applicable cases in JDK8. Naoto
[8u] RFR: 8177776: Create an equivalent test case for JDK9's SupplementalJapaneseEraTest
Hello, Please review the changes to the following issue: https://bugs.openjdk.java.net/browse/JDK-816 The proposed change is located at: http://cr.openjdk.java.net/~naoto/816/webrev.00/ This is for backporting fixes for JapaneseEra related issues (8054214, 8173423). The original fixes in JDK9 included a test case, SupplementalJapaneseEraTest which is intended for the system property testing. The above proposed fix intends to adapt that test case into JDK8, where calendars.properties file is used instead of the system property. The test is pretty much identical to JDK9's. The difference is to deal with the calendars.properties file and removed some non-applicable cases in JDK8. Naoto
