Need reviewer - jdi tests fixes

Kelly O'Hair Kelly.Ohair at Sun.COM
Tue Nov 24 15:36:47 PST 2009


Daniel D. Daugherty wrote:
> Kelly O'Hair wrote:
>> Need reviewer on changes to the com/sun/jdi tests.
>>
>> 6904183: Fix jdk/test/com/sun/jdi tests to run with -samevm
>> http://cr.openjdk.java.net/~ohair/openjdk7/jdk7-tl-test-changes/webrev/
>>
>> The com/sun/jdi tests can run with 'jtreg -samevm' with these changes.
>> Although not all these changes had to do with -samevm.
>>
>> Multiple issues fixed:
>>   * Solaris amd64 not being tested in multiple places
>>   * Assumptions that the current directory was also the classes dir
>>     (Added "-classpath test.classes" in multiple places).
>>   * Made a few tests permanently 'othervm'
>>   * Removed SimulResumerTest.java and RedefineException.sh from the
>>     problem list for now. (No jdi tests would be on the problem list)
> 
> None of these comments are "must do".

Ok. Thanks.

> 
> test/Makefile
>    No comments.
> 
> test/ProblemList.txt
>    Why comment out the entries instead of removing them?

I'll remove them as soon as I'm sure they run ok now.
It needs a few passes through JPRT before I'll be convinced
they are ok.

> 
> test/com/sun/jdi/BadHandshakeTest.java
>    This alternate architecture stuff is screaming for a
>    refactoring. Can you file a new bug to document that
>    we need to do that?

Ok. Will do. I'll make it P4, not sure when anyone will have time
for it.  Filed 6904593.

> 
> test/com/sun/jdi/DoubleAgentTest.java
>    No comments.
> 
> test/com/sun/jdi/ExclusiveBind.java
>    No comments.
> 
> test/com/sun/jdi/JITDebug.sh
>    No comments.
> 
> test/com/sun/jdi/RepStep.java
>    You might want to add a comment about why this has
>    to be an "othervm" test.

I honestly am not sure, after making the basic changes for
classpath, I could not figure out why this test was problematic,
adding othervm made it pass. Some of these tests are not
horribly straight forward, or my mind has turned to mush because
I can't figure some of them out at times. :^(
So any comment I could add would be less that trustworthy,
and it doesn't seem to be worth a great deal of time to figure
out why.

I've seen many tests like this, pass with othervm, don't
with samevm, if it's not obvious, I just mark it othervm.
Or if I suspect this test is polluting the state of the 'samevm'
and making other tests fail, I'll just mark it othervm.

> 
> test/com/sun/jdi/RunToExit.java
>    You might want to add a comment about why this has
>    to be an "othervm" test. Although, if the test name
>    is a good clue, then...

Dito, although I kind of assumed it was a wise thing to make
this one othervm. ;^)

> 
> test/com/sun/jdi/SimulResumerTest.java
>    You might want to add a comment about why this has
>    to be an "othervm" test.

Dito. Don't know for sure.

> 
> test/com/sun/jdi/Solaris32AndSolaris64Test.sh
>    No comments.
> 
> test/com/sun/jdi/VMConnection.java
>    No comments.
> 
> test/com/sun/jdi/connect/spi/DebugUsingCustomConnector.java
>    You might want to add a comment about why this has
>    to be an "othervm" test.

Dito.

> 
> test/com/sun/jdi/connect/spi/GeneratedConnectors.java
>    You might want to add a comment about why this has
>    to be an "othervm" test.

Dito.

> 
> test/com/sun/jdi/connect/spi/SimpleLaunchingConnector.java
>    Did the original test really have an empty string as
>    part of the command?

Yes it did. :^(  Looks like it glued the classname to the
address, I don't understand how this ever worked.


> 
> test/com/sun/jdi/redefine/RedefineTest.java
>    You might want to add a comment about why this has
>    to be an "othervm" test. Although, I suspect that all
>    "redefine" tests have to be "othervm".

I just assumed that it needed to be othervm. No exact reasons
other than the behavior of the test.
But the jury is still out on this one, I may need to work on
it or put it back on the problem list, we'll see.

-kto


More information about the serviceability-dev mailing list