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

[Mikael Gerdin]

Leonid,

On 2013-04-04 12:34, Leonid Mesnik wrote:
> Mikael
>
> Here is updated webrev:
> http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.2
>
> Streams were redirected, toArray() was updated.

Looks good to me.

/Mikael

>
> Leonid
> On 04/02/2013 01:57 PM, Mikael Gerdin wrote:
>> 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
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>
>>>
>
>