<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=ISO-8859-1">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
Including the correct email list.<br>
<br>
Bengt<br>
<div class="moz-forward-container"><br>
<br>
-------- Original Message --------
<table class="moz-email-headers-table" border="0" cellpadding="0"
cellspacing="0">
<tbody>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject:
</th>
<td>Re: Fwd: Re: RFR: 8009808 TEST-BUG : test case is using
bash style tests. Default shell for jtreg is bourne. thus
failure</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date: </th>
<td>Tue, 09 Apr 2013 14:40:09 +0200</td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">From: </th>
<td>Bengt Rutisson <a class="moz-txt-link-rfc2396E" href="mailto:bengt.rutisson@oracle.com"><bengt.rutisson@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th>
<td>Mikael Gerdin <a class="moz-txt-link-rfc2396E" href="mailto:mikael.gerdin@oracle.com"><mikael.gerdin@oracle.com></a></td>
</tr>
<tr>
<th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th>
<td>hotspot_gc_staff_ww_grp
<a class="moz-txt-link-rfc2396E" href="mailto:hotspot_gc_staff_ww_grp@oracle.com"><hotspot_gc_staff_ww_grp@oracle.com></a></td>
</tr>
</tbody>
</table>
<br>
<br>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
<div class="moz-cite-prefix"><br>
Leonid,<br>
<br>
I think the test looks good.<br>
<br>
One minor nit: I think the the @summary comment could maybe be a
bit different. It says:<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
@summary test new added flags for gc log rotation<br>
<br>
In a while these flags won't be newly added. I think it is
enough to say:<br>
<br>
@summary test flags for gc log rotation<br>
<br>
Thanks,<br>
Bengt<br>
<br>
<br>
On 4/9/13 1:45 PM, Mikael Gerdin wrote:<br>
</div>
<blockquote cite="mid:5163FF59.1000201@oracle.com" type="cite">Can
one of our Reviewers please look at this, it's a good cleanup of
a messy test for gc log rotation. <br>
<br>
/m <br>
<br>
-------- Original Message -------- <br>
Subject: Re: RFR: 8009808 TEST-BUG : test case is using bash
style tests. Default shell for jtreg is bourne. thus failure <br>
Date: Mon, 08 Apr 2013 18:23:10 +0400 <br>
From: Leonid Mesnik <a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="mailto:leonid.mesnik@oracle.com"><leonid.mesnik@oracle.com></a>
<br>
CC: <a moz-do-not-send="true" class="moz-txt-link-abbreviated"
href="mailto:hotspot-gc-dev@openjdk.java.net">hotspot-gc-dev@openjdk.java.net</a>
<a moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="mailto:hotspot-gc-dev@openjdk.java.net"><hotspot-gc-dev@openjdk.java.net></a>
<br>
<br>
I still need approval from reviewers for this fix. <br>
<br>
Could anyone take a look on it? <br>
<br>
Leonid <br>
On 04/04/2013 03:57 PM, Mikael Gerdin wrote: <br>
<blockquote type="cite">Leonid, <br>
<br>
On 2013-04-04 12:34, Leonid Mesnik wrote: <br>
<blockquote type="cite">Mikael <br>
<br>
Here is updated webrev: <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.2">http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.2</a>
<br>
<br>
Streams were redirected, toArray() was updated. <br>
</blockquote>
<br>
Looks good to me. <br>
<br>
/Mikael <br>
<br>
<blockquote type="cite"> <br>
Leonid <br>
On 04/02/2013 01:57 PM, Mikael Gerdin wrote: <br>
<blockquote type="cite">Leonid, <br>
<br>
On 2013-03-29 10:38, Leonid Mesnik wrote: <br>
<blockquote type="cite">Hi <br>
<br>
Here is new version of test. (pass jprt) <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.1/">http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.1/</a>
<br>
<a moz-do-not-send="true" class="moz-txt-link-rfc2396E"
href="http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.1/"><http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.1/></a>
<br>
</blockquote>
<br>
Overall it looks good. The test is nicely specified and in
a <br>
controlled environment. <br>
<br>
Can you please make the test save the stdout/stderr of the
launched <br>
process and print it out in case of failure? <br>
If the launch fails with some exit code we won't know its
cause. I <br>
suggest something like: <br>
pb.redirectErrorStream(true) <br>
pb.redirectOutput("output.log") <br>
<br>
I also have one small comment in: <br>
args.toArray(externalVMopts) <br>
<br>
Even though it may be correct to pass exernalVMopts to
toArray(T[]) it <br>
looks a bit confusing. <br>
Can you please change it to either <br>
"toArray(new String[0])" or <br>
"toArray(new String[args.size()])" <br>
<br>
/Mikael <br>
<br>
<blockquote type="cite"> <br>
See my comment inline. <br>
On 03/26/2013 05:30 PM, Mikael Gerdin wrote: <br>
<blockquote type="cite">Leonid, <br>
<br>
On 2013-03-22 08:06, Leonid Mesnik wrote: <br>
<blockquote type="cite">Could anyone please review
this small test fix. <br>
<br>
Leonid <br>
On 03/20/2013 04:16 PM, Leonid Mesnik wrote: <br>
<blockquote type="cite">Hi <br>
<br>
Could you please review fix for 8009808 TEST-BUG
: test case is <br>
using <br>
bash style tests. Default shell for jtreg is
bourne. thus failure. <br>
<br>
I've completely rewritten test in java without
major changes it <br>
test <br>
logic. <br>
I remove CMS so test could be run when CMS is not
supported. Also I <br>
changed max memory to 128M to reduce memory load
and increase <br>
number <br>
of GC log entries. <br>
</blockquote>
</blockquote>
<br>
Do you think it would be useful to run this test with
different GCs? <br>
In that case you can pick up the test.vm.opts and
test.java.opts <br>
system properties and add them to the java command
line you launch. <br>
</blockquote>
I've added test.java.opts properties. They are used
during testing to <br>
set additional GC. <br>
<blockquote type="cite"> <br>
<blockquote type="cite">
<blockquote type="cite"> <br>
Here is the link: <br>
<a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.0/">http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.0/</a>
<br>
<a moz-do-not-send="true"
class="moz-txt-link-rfc2396E"
href="http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.0/"><http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.0/></a>
<br>
<br>
<br>
<br>
</blockquote>
</blockquote>
<br>
Are you sure about this: <br>
static final File currentDirectory = new <br>
File(System.getProperty("user.dir")); <br>
<br>
isn't user.dir the home directory? Current directory
should be <br>
just "." <br>
Something like new File(".").getAbsoluteFile() should
give you the <br>
current work dir. <br>
</blockquote>
System.getProperty("user.dir") is current directory.
However I changed <br>
it to "." to make it more evident. <br>
<blockquote type="cite"> <br>
What is the relation between "numberOfFiles" and
"minutesToRun"?? <br>
How do you know that running for 3 minutes will cause
a log rotation? <br>
</blockquote>
I've updated test to invoke fullGC and estimate lower
bound of needed <br>
invocation. Now test check that exactly 3 logs are
generated. <br>
<br>
Leonid <br>
<blockquote type="cite"> <br>
I know that you've just adapted the existing test but
it seems <br>
strange <br>
to me. <br>
<br>
/Mikael <br>
<br>
<br>
<br>
<blockquote type="cite"> <br>
<br>
</blockquote>
</blockquote>
<br>
<br>
</blockquote>
</blockquote>
<br>
<br>
</blockquote>
</blockquote>
<br>
<br>
</blockquote>
<br>
<br>
</div>
<br>
</body>
</html>