[Fwd: SDK Test Fixes Batch for 2010.07 (6941287, 6962804, 6964018)]
David.Holmes at oracle.com
Wed Jul 21 16:41:38 PDT 2010
You can add me to the first two if you like. The third was over my head :)
As Alan said these are test changes so not so critical ... and if
there's a problem with them you get to fix it again :)
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 appreciated...
>>>> 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 removed.
> 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