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

[Leonid Mesnik]

I still need approval from reviewers for this fix.

Could anyone take a look on it?

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


-- 
Leonid Mesnik
JVM SQE