[Fwd: SDK Test Fixes Batch for 2010.07 (6941287, 6962804, 6964018)]
Daniel D. Daugherty
daniel.daugherty at oracle.com
Wed Jul 21 16:43:59 PDT 2010
On 7/21/2010 5:41 PM, David Holmes wrote:
> You can add me to the first two if you like.
> The third was over my head :)
I doubt that, but I have two for that one :-)
> As Alan said these are test changes so not so critical ...
Unless you're trying to get an SDK/JDK job through the JPRT queue :-)
> and if there's a problem with them you get to fix it again :)
Yup. And now I have a template/shared code to use to fix other
annoyingly intermittent tests...
> Daniel D. Daugherty said the following on 07/22/10 06:12:
>> I'm adding the OpenJDK aliases back into this e-mail thread for
>> this reply. Just trying to keep folks in the loop...
>> Thanks for squeezing in time for a review of 6964018.
>> On 7/21/2010 1:43 PM, Alan Bateman wrote:
>>> Daniel D. Daugherty wrote:
>>>> On 7/21/2010 4:42 AM, Alan Bateman wrote:
>>>>> Daniel D. Daugherty wrote:
>>>>>> A quick look from you at the third webrev would be much
>>>>> Sorry Dan, I just don't have the cycles today due to a couple of
>>>>> high priority issues. I might get cycles on Friday if we really
>>>>> want me to review.
>>>> I may just go with only Kelly's review. :-|
>>> I just spent 15-20m going through
>>> http://cr.openjdk.java.net/~dcubed/6964018-webrev/0/ in more detail
>>> and it looks fine to me. Thanks for explaining about the refactoring
>>> as it looks more than it really is on first glance. I'm happy with
>>> the updates to the existing tests and happy to see the new
>>> test/java/util/logging tests are more reliable. Given that this is
>>> tests only change then one reviewer should be fine.
>> I'll put both you and Kelly down for 6964018. I'll put just Kelly
>> down for the first two. Of course, that could change if I get more
>> reviewer comments.
>>> Minor nits in ShutdownSimpleApplication.java is that System.exit is
>>> probably not needed.
>> Because I added the 'set -e' option, I think it is needed. Otherwise
>> I get a random process exit status.
>>> Also the pre-existing empty comment lines L25-26 can probably be
>> I wondered why those were there so I left them in. I had meant to
>> ping you about them, but I forgot... I'll remove them.
>>> Same comments for SimpleApplication.
>> Hopefully, the same resolutions will be OK with you.
>>> BTW: In test/java/util/logging/LoggerWeakRefLeak.sh you can listed
>>> issues with jmap - are they still valid with the updated test and
>>> the recent fix to the attach API?
>> I suspect that your recent attach API fix will resolve those
>> issues, but I plan to specifically test for that after your
>> fix gets promoted.
>>> The only outstanding issue that I can think of is where you try to
>>> attach just as the process is about to exit (but with the updated
>>> test this doesn't happen due to the "handshake"/socket connection).
>> Agreed and thank you for pointing me in that direction.
More information about the hotspot-runtime-dev