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

[Leonid Mesnik]

Mikael

Here is updated webrev:
http://cr.openjdk.java.net/~mgerdin/lmesnik/log_rot_test/webrev.2

Streams were redirected, toArray() was updated.

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
>>>
>>>
>>>
>>>>
>>>>
>>
>>


-- 
Leonid Mesnik
JVM SQE