<AWT Dev> [8] Review request for 6868690 TEST: java/awt/FontClass/CreateFont/BigFont.java test should be modified in jdk7&8 to run via jtreg

[Konstantin Shefov]

Hello, Artem

I commented inline.

On 02.07.2012 23:23, Artem Ananiev wrote:
> Hi, Konstantin,
>
> see my comments inline.
>
> On 6/13/2012 4:09 PM, Konstantin Shefov wrote:
>> Hello,
>>
>> I have decided to remove bigfont.html and turn an applet test into main
>> test, because applet test always fails with timeout.
>
> Could you clarify, how changing applet->main helps with the timeout? 
> If the test should be run with a greater timeout, it can be specified 
> as a modifier of @run tag.
>
> In the current version of the fix, BigFont.runTest2() is never called, 
> and there is no .html file to run the test as applet. Have you had a 
> chance to look at my proposal with
>
>   @run main
>
> in BigFont.java and
>
>   @run applet/manual
>
> in bigfont.html?

If we run this test as applet in jtreg (with appletviewer), it just 
hangs and never finishes (I waited 24 hours - no effect, it hangs). The 
test hangs and all testsuite run hangs. If we run  BigFont.runTest2() 
manually with browser (not jtreg, nor appletviewer) with file A.ttf, we 
should wait about 8 hours for the test to complete (file A.ttf is very 
small for this test).
In jdk 6 and 7 we already have a fix pushed without applet and 
BigFont.runTest2() method at all (although bigfont.html remained in 
testsuite as just a file, it is not used there).

>
>> I have left method "runTest2()" commented because it requires rather
>> large font file (included
>> http://hg.openjdk.java.net/jdk8/awt/jdk/file/b57167b71169/test/java/awt/FontClass/CreateFont/A.ttf 
>>
>> font is a small file).
>> I have added fix for
>> test/java/awt/FontClass/CreateFont/fileaccess/FontFile.java as it was
>> done in jdk 7u4b11, and the test always passed with jdk 8 (it is also a
>> part of bug 6868690).
>
> What is the reason of having shell test here? The test explicitly 
> checks that access to certain file is protected and that we get 
> IOException for non-existing file.
>
In fact this is a fix for bug 6802962, but there is no Multiple Release 
entry for jdk8 and it is discussed in CR 6868690. In the original state 
the test is a stopper for a whole testing session if executed under 
jtreg. It was modified to resolve this issue. The main reason is the 
system class loader is initialized at a very early point in the startup 
sequence. It copies the classpath into its own data structures, and the 
classpath property is not read again. For the particular test (when we 
run it under jtreg) the jtreg invokes the own main method and the 
codebase (not the codebase of the test class file location) that should 
work for this case. This is the know point. A wrapper file is a solution 
the is normally used for jtreg based test Workspaces. The shell script 
wrapper is used.

This fix was integrated in jdk 7 ws already.

> Thanks,
>
> Artem
>
>> Here is the webrev:
>> http://cr.openjdk.java.net/~kshefov/6868690/webrev.00/
>> <http://cr.openjdk.java.net/%7Ekshefov/6868690/webrev.00/>
>>
>> Thanks,
>> Konstantin
>>
>> On 08.06.2012 16:07, Artem Ananiev wrote:
>>>
>>> On 6/8/2012 12:54 PM, Konstantin Shefov wrote:
>>>> Artem,
>>>>
>>>> I think there are two ways:
>>>> 1) create one manual test with instructions to run applet (html) in
>>>> browser + one automatic test without html.
>>>> Method runTest2() should be used only within applet.
>>>
>>> OK, it can be implemented by having
>>>
>>>     @run applet/manual
>>>
>>> in .html file, and
>>>
>>>     @run main
>>>
>>> in .java file.
>>>
>>> Thanks,
>>>
>>> Artem
>>>
>>>> 2) remove html and runTest2() method at all, so only one automatic 
>>>> test
>>>> will remain.
>>>>
>>>> Konstantin
>>>>
>>>> On 08.06.2012 13:19, Artem Ananiev wrote:
>>>>> Hi, Konstantin,
>>>>>
>>>>> did you consider the following option:
>>>>>
>>>>> 1. Remove .html file
>>>>>
>>>>> 2. Add one more configuration:
>>>>>
>>>>> @run main BigFont test1
>>>>> @run main/manual BigFont test1 test2
>>>>>
>>>>> 3. Update the "main" method, so it recognizes different command line
>>>>> args:
>>>>>
>>>>> for (String arg: args)
>>>>> if (arg.equalsIgnoreCase("test1")
>>>>> bigTest.runTest1();
>>>>> else if (arg.equalsIgnoreCase("test2")
>>>>> bigTest.runTest2();
>>>>>
>>>>> The reason I'm asking about that even for manual runs people still 
>>>>> use
>>>>> "jtreg", just passing "-m" to this script. As you have removed @run
>>>>> from the .html file, it will not be recognized by jtreg at all.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Artem
>>>>>
>>>>> On 6/4/2012 5:32 PM, Konstantin Shefov wrote:
>>>>>> Hi, Artem
>>>>>>
>>>>>> As I have mentioned in review description, we can keep .html file 
>>>>>> and
>>>>>> runTest2() method commented in the BigFont.java if somebody 
>>>>>> wishes to
>>>>>> run it manually (e.g. when a failure happens). The .html file and 
>>>>>> the
>>>>>> method were kept in openjdk 7 regression testsuite. Or you think we
>>>>>> don't need to keep it?
>>>>>>
>>>>>> Konstantin
>>>>>>
>>>>>> On 04.06.2012 17:24, Artem Ananiev wrote:
>>>>>>> Hi, Konstantin,
>>>>>>>
>>>>>>> since you have replaced "@run applet" in .html with "@run main" in
>>>>>>> .java, does it also make sense to remove .html at all? 
>>>>>>> Alternatively,
>>>>>>> you can keep the test as applet one, but mark it manual (@run
>>>>>>> applet/manual).
>>>>>>>
>>>>>>> BTW, copyright header dates should be updated to "2008, 2012", not
>>>>>>> "2008, 2011".
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Artem
>>>>>>>
>>>>>>> On 5/31/2012 5:59 PM, Konstantin Shefov wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> Please review a fix for the issue:
>>>>>>>> 6868690 TEST: java/awt/FontClass/CreateFont/BigFont.java test
>>>>>>>> should be
>>>>>>>> modified in jdk7&8 to run via jtreg
>>>>>>>>
>>>>>>>> The webrev is http://cr.openjdk.java.net/~serb/6868690/webrev.00/
>>>>>>>>
>>>>>>>> The test in the current state affects a whole session if it's
>>>>>>>> executed
>>>>>>>> under jtreg. It was fixed already by the implemented
>>>>>>>> modifications in
>>>>>>>> 1.7.0 that are used as a base for modifications applicable for 
>>>>>>>> 1.8.0
>>>>>>>> also. There is not risk are seen if the test be updated in jdk8
>>>>>>>> repo.
>>>>>>>>
>>>>>>>> The current fix is identical to already pushed fix for jdk 7.
>>>>>>>>
>>>>>>>> The test works in the automatic invocation (under jtreg) without
>>>>>>>> bigfont.html file involvement. The only possible reason we may
>>>>>>>> keep it
>>>>>>>> (bigfont.html) for is - the test can be used in both the manual 
>>>>>>>> and
>>>>>>>> automatic invocations. It include two methods runTest1() &
>>>>>>>> runTest2().
>>>>>>>> runTest2() is commented in the BigFont.java file as it's too
>>>>>>>> "risky" to
>>>>>>>> execute it in the automatic mode from the the applet environment.
>>>>>>>> But it
>>>>>>>> (runTest2()) is still valid for the manual invocation just for
>>>>>>>> "legacy/coverage" reason to be executed under applet environment
>>>>>>>> that is
>>>>>>>> more challenging. As mentioned bigfont.html does not work in
>>>>>>>> automatic
>>>>>>>> invocation, but it can work if somebody decide to run the test
>>>>>>>> manually
>>>>>>>> (runTest2()) under applet environment.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Konstantin
>>>>>>>>