code review for attach on demand (AOD) test fix (7035555)

[Dmitry Samersoff]

Dan,

test/com/sun/tools/attach/ApplicationSetup.sh

I don't know whether you use the same shell script for UNIX and Cygwin
but I think

"${TESTCLASSES}/Application.jar"

is safer than

"${TESTCLASSES}"/Application.jar


test/com/sun/tools/attach/BasicTests.java

Nit:

+  System.out.println("INFO: SilverBullet.jar not being found and an");
+  System.out.println("INFO: agent failing to start.");

It might be better to keep article with a word. i.e

+  System.out.println("INFO: SilverBullet.jar not being found and ");
+  System.out.println("INFO: an agent failing to start.");

The same for

+System.out.println("INFO: appear in the application log about a");
+System.out.println("INFO: BadAgent including a RuntimeException and");


test/com/sun/tools/attach/BasicTests.sh

if [ echo ${osrev} | grep -q 'CYGWIN[^ ]*-5\.0' ] ; then

IMHO, is better way to use grep but probably you don't need to change it.

-Dmitry



On 2011-04-12 19:12, Daniel D. Daugherty wrote:
> Greetings,
>
> I have minor fixes to an Attach On Demand (AOD) test that I'd
> like to get into T&L snapshot for OpenJDK7-B140 (next week).
> Here is my proposed changeset comment:
>
> 7035555: 4/4 attach/BasicTests.sh needs another tweak for Cygwin
> Summary: Test needs to properly detect missing
> AgentInitializationException. Clarify when exceptions
> are expected. Another Cygwin tweak.
>
> Yes, this bug started out as a tweak for Cygwin and then I
> discovered and fixed the other small issues.
>
> Here is the URL to the webrev:
>
> http://cr.openjdk.java.net/~dcubed/7035555-webrev/0/
>
> I've run the fix through JPRT testing and I ran into an existing
> intermittent failure:
>
> 6461635 4/3 BasicTests.sh test fails intermittently.
>
> I've checked nightly testing and 6461635 makes a periodic
> appearance across all platforms.
>
> Thanks, in advance, for any comments.
>
> Dan
>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...