RFR: 8009808 TEST-BUG : test case is using bash style tests. Default shell for jtreg is bourne. thus failure

[Mikael Gerdin]

Leonid,

On 2013-03-29 10:38, Leonid Mesnik wrote:
> Hi
>
> Here is new version of test. (pass jprt)
> http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.1/
> <http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.1/>

Overall it looks good. The test is nicely specified and in a controlled 
environment.

Can you please make the test save the stdout/stderr of the launched 
process and print it out in case of failure?
If the launch fails with some exit code we won't know its cause. I 
suggest something like:
pb.redirectErrorStream(true)
pb.redirectOutput("output.log")

I also have one small comment in:
args.toArray(externalVMopts)

Even though it may be correct to pass exernalVMopts to toArray(T[]) it 
looks a bit confusing.
Can you please change it to either
"toArray(new String[0])" or
"toArray(new String[args.size()])"

/Mikael

>
> See my comment inline.
> On 03/26/2013 05:30 PM, Mikael Gerdin wrote:
>> Leonid,
>>
>> On 2013-03-22 08:06, Leonid Mesnik wrote:
>>> Could anyone please review this small test fix.
>>>
>>> Leonid
>>> On 03/20/2013 04:16 PM, Leonid Mesnik wrote:
>>>> Hi
>>>>
>>>> Could you please review fix for  8009808 TEST-BUG : test case is using
>>>> bash style tests. Default shell for jtreg is bourne. thus failure.
>>>>
>>>> I've completely rewritten test in java without major changes it test
>>>> logic.
>>>> I remove CMS so test could be run when CMS is not supported. Also I
>>>> changed max memory to 128M to reduce memory load and increase number
>>>> of GC log entries.
>>
>> Do you think it would be useful to run this test with different GCs?
>> In that case you can pick up the test.vm.opts and test.java.opts
>> system properties and add them to the java command line you launch.
> I've added test.java.opts properties. They are used during testing to
> set additional GC.
>>
>>>>
>>>> Here is the link:
>>>> http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.0/
>>>> <http://cr.openjdk.java.net/%7Emgerdin/lmesnik/log_rot_test/webrev.0/>
>>>>
>>
>> Are you sure about this:
>> static final File currentDirectory = new
>> File(System.getProperty("user.dir"));
>>
>> isn't user.dir the home directory? Current directory should be just "."
>> Something like new File(".").getAbsoluteFile() should give you the
>> current work dir.
> System.getProperty("user.dir") is current directory. However I changed
> it to "." to make it more evident.
>>
>> What is the relation between "numberOfFiles" and "minutesToRun"??
>> How do you know that running for 3 minutes will cause a log rotation?
> I've updated test to invoke fullGC and estimate lower bound of needed
> invocation. Now test check that exactly 3 logs are generated.
>
> Leonid
>>
>> I know that you've just adapted the existing test but it seems strange
>> to me.
>>
>> /Mikael
>>
>>
>>
>>>
>>>
>
>