MI-related testsuite regressions

MI-related testsuite regressions

[Mark Kettenis]

Nick,

I see a fair number of new testsuite regressions:

-PASS: gdb.mi/mi2-stack.exp: stack select same frame
+FAIL: gdb.mi/mi2-stack.exp: stack select same frame

-PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
-PASS: gdb.mi/mi2-var-block.exp: create local variable foo2
+FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
+FAIL: gdb.mi/mi2-var-block.exp: create local variable foo2

-PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
+FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)

-PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
+FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)

-PASS: gdb.mi/mi2-var-block.exp: update cb
+FAIL: gdb.mi/mi2-var-block.exp: update cb

-PASS: gdb.mi/mi2-var-block.exp: delete var foo2
+FAIL: gdb.mi/mi2-var-block.exp: delete var foo2

They seem all to be related to your recent changes.  Please make sure
you run the (complete) testsuite and check for regressions before you
check things in.  I usually keep an old gdb.sum around and diff it
with the new output to check for regressions.

Mark

MI-related testsuite regressions

[Nick Roberts]

 > I see a fair number of new testsuite regressions:
 > 
 > -PASS: gdb.mi/mi2-stack.exp: stack select same frame
 > +FAIL: gdb.mi/mi2-stack.exp: stack select same frame

Yes.  These are due to my changes.  Currently the default MI interpreter *is*
mi2, so these tests are just duplicated.  It would seem sensible, and save
effort, to have just one file (mi-stack.exp) until the MI version is bumped
up one number.  Then mi-stack.exp could be renamed mi2-stack.exp and a new
file mi-stack.exp could be created.

In reality GDB doesn't support more than one version of MI (the current one).
As has been shown on the mailing list recently, even MI output from GDB 6.3
differs from MI output from GDB in CVS.

 > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
 > -PASS: gdb.mi/mi2-var-block.exp: create local variable foo2
 > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
 > +FAIL: gdb.mi/mi2-var-block.exp: create local variable foo2
 > 
 > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
 > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
 > 
 > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
 > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
 > 
 > -PASS: gdb.mi/mi2-var-block.exp: update cb
 > +FAIL: gdb.mi/mi2-var-block.exp: update cb
 > 
 > -PASS: gdb.mi/mi2-var-block.exp: delete var foo2
 > +FAIL: gdb.mi/mi2-var-block.exp: delete var foo2

> They seem all to be related to your recent changes.

I can't see how these relate to my changes.  I have submitted a patch for
variable objects, but it still hasn't been approved.  I don't understand why
mi-var-block.exp doesn't fail at the same places.

> Please make sure you run the (complete) testsuite and check for regressions
> before you check things in.  I usually keep an old gdb.sum around and diff it
> with the new output to check for regressions.

Sure.  I get all kinds of spurious fails on my old computer which makes this a
bit harder to do.

Nick

Re: MI-related testsuite regressions

[Daniel Jacobowitz]

On Tue, Jun 28, 2005 at 12:01:36AM +1200, Nick Roberts wrote:
>  > I see a fair number of new testsuite regressions:
>  > 
>  > -PASS: gdb.mi/mi2-stack.exp: stack select same frame
>  > +FAIL: gdb.mi/mi2-stack.exp: stack select same frame
> 
> Yes.  These are due to my changes.

I'm going to echo Mark here.  I assumed that you would not post a patch
which introduced failures in the testsuite.

>  Currently the default MI interpreter *is*
> mi2, so these tests are just duplicated.  It would seem sensible, and save
> effort, to have just one file (mi-stack.exp) until the MI version is bumped
> up one number.  Then mi-stack.exp could be renamed mi2-stack.exp and a new
> file mi-stack.exp could be created.
> 
> In reality GDB doesn't support more than one version of MI (the current one).
> As has been shown on the mailing list recently, even MI output from GDB 6.3
> differs from MI output from GDB in CVS.

mi2 is a released protocol; the intent is that we not make
backwards-incompatible changes to mi2-*.exp, at least not without
paying close attention to them.  -i=mi sets mi_version to 3 today. 
Until we're ready to declare mi3 usable, we need to continue caring
about mi2.

You removed the parallel to this failing test from mi-stack.exp; I
assume it should be removed from mi2-stack.exp also?

>  > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
>  > -PASS: gdb.mi/mi2-var-block.exp: create local variable foo2
>  > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
>  > +FAIL: gdb.mi/mi2-var-block.exp: create local variable foo2
>  > 
>  > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
>  > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
>  > 
>  > -PASS: gdb.mi/mi2-var-block.exp: step at do_block_test
>  > +FAIL: gdb.mi/mi2-var-block.exp: step at do_block_test (stopped at wrong place)
>  > 
>  > -PASS: gdb.mi/mi2-var-block.exp: update cb
>  > +FAIL: gdb.mi/mi2-var-block.exp: update cb
>  > 
>  > -PASS: gdb.mi/mi2-var-block.exp: delete var foo2
>  > +FAIL: gdb.mi/mi2-var-block.exp: delete var foo2
> 
> > They seem all to be related to your recent changes.
> 
> I can't see how these relate to my changes.  I have submitted a patch for
> variable objects, but it still hasn't been approved.  I don't understand why
> mi-var-block.exp doesn't fail at the same places.

Mark, could you post a bit of gdb.log?  I don't see these.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: MI-related testsuite regressions

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Tue, Jun 28, 2005 at 12:01:36AM +1200, Nick Roberts wrote:
 > >  > I see a fair number of new testsuite regressions:
 > >  > 
 > >  > -PASS: gdb.mi/mi2-stack.exp: stack select same frame
 > >  > +FAIL: gdb.mi/mi2-stack.exp: stack select same frame
 > > 
 > > Yes.  These are due to my changes.
 > 
 > I'm going to echo Mark here.  I assumed that you would not post a patch
 > which introduced failures in the testsuite.

Its not a regression.  I just removed the test from mi-stack.exp as it was
no longer appropriate.  I attach a patch below which also adds the test for
-stack-info-frame and for "-stack-list-locals 2" (which I added in January
of last year) from mi-stack.exp.  I can't test them at the moment but they've
just been copied so they should be OK.

 > >  Currently the default MI interpreter *is*
 > > mi2, so these tests are just duplicated.  It would seem sensible, and save
 > > effort, to have just one file (mi-stack.exp) until the MI version is bumped
 > > up one number.  Then mi-stack.exp could be renamed mi2-stack.exp and a new
 > > file mi-stack.exp could be created.
 > > 
 > > In reality GDB doesn't support more than one version of MI (the current
 > > one).  As has been shown on the mailing list recently, even MI output
 > > from GDB 6.3 differs from MI output from GDB in CVS.
 > 
 > mi2 is a released protocol; the intent is that we not make
 > backwards-incompatible changes to mi2-*.exp, at least not without
 > paying close attention to them.  -i=mi sets mi_version to 3 today. 
 > Until we're ready to declare mi3 usable, we need to continue caring
 > about mi2.

-i=mi sets mi_version to 2 in my copy:

  interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs));

and

  if (current_interp_named_p (INTERP_MI1))
    deprecated_command_loop_hook = mi1_command_loop;
  else if (current_interp_named_p (INTERP_MI2))
    deprecated_command_loop_hook = mi2_command_loop;
  else if (current_interp_named_p (INTERP_MI3))
    deprecated_command_loop_hook = mi3_command_loop;
  else
    deprecated_command_loop_hook = mi2_command_loop;


As far as I can see mi3 does nothing that mi2 doesn't do.

Nick


2005-06-28  Nick Roberts  <nickrob@snap.net.nz>

	* gdb.mi/mi-stack.exp (test_stack_locals_listing): Remove test for
	-stack-select-frame without arguments.  Add test for newly
	implemented command -stack-info-frame.
	(test_stack_locals_listing): Test for case "-stack-list-locals 2".


*** gdb.mi/mi2-stack.exp.~1.4.~	2005-05-18 20:18:15.000000000 +1200
--- gdb.mi/mi2-stack.exp	2005-06-28 01:29:40.000000000 +1200
***************
*** 55,60 ****
--- 55,61 ----
      # -stack-list-frames
      # -stack-list-frames 1 1
      # -stack-list-frames 1 3
+     # -stack-info-frame
  
      mi_gdb_test "231-stack-list-frames" \
  	    "231\\^done,stack=\\\[frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\},frame=\{level=\"1\",addr=\"$hex\",func=\"callee3\",.*\},frame=\{level=\"2\",addr=\"$hex\",func=\"callee2\",.*\},frame=\{level=\"3\",addr=\"$hex\",func=\"callee1\",.*\},frame=\{level=\"4\",addr=\"$hex\",func=\"main\",.*\}\\\]" \
***************
*** 69,74 ****
--- 70,79 ----
      mi_gdb_test "234-stack-list-frames 1" \
  	    "234\\^error,msg=\"mi_cmd_stack_list_frames: Usage.*FRAME_LOW FRAME_HIGH.*\"" \
  	    "stack frame listing wrong"
+ 
+     mi_gdb_test "235-stack-info-frame" \
+             "235\\^done,frame=\{level=\"0\",addr=\"$hex\",func=\"callee4\",file=\".*basics.c\",fullname=\"${fullname_syntax}${srcfile}\",line=\"$line_callee4_body\"\}" \
+ 	       "selected frame listing"
  }
  
  proc test_stack_args_listing {} {
***************
*** 149,154 ****
--- 154,160 ----
      # Tests:
      # -stack-list-locals 0
      # -stack-list-locals 1
+     # -stack-list-locals 2
      # -stack-list-arguments 
  
      mi_gdb_test "232-stack-list-locals 0" \
***************
*** 170,175 ****
--- 176,185 ----
  	    "232\\^done,locals=\\\[\{name=\"A\",value=\"1\"\},\{name=\"B\",value=\"2\"\},\{name=\"C\",value=\"3\"\}\\\]" \
                  "stack locals listing 1"
  
+     mi_gdb_test "232-stack-list-locals 2" \
+ 	    "232\\^done,locals=\\\[\{name=\"A\",type=\"int\",value=\"1\"\},\{name=\"B\",type=\"int\",value=\"2\"\},\{name=\"C\",type=\"int\",value=\"3\"\}\\\]" \
+                 "stack locals listing 2"
+ 
      mi_gdb_test "234-stack-list-locals" \
  	    "234\\^error,msg=\"mi_cmd_stack_list_locals: Usage.*PRINT_VALUES.*\"" \
  	    "stack locals listing wrong"
***************
*** 182,197 ****
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals listing for new frame"
  
- # this should be a no-op
- 
-     mi_gdb_test "232-stack-select-frame" \
- 	    "232\\^done" \
-                 "stack select same frame"
- 
      mi_gdb_test "232-stack-list-locals 1" \
  	    "232\\^done,locals=\\\[\\\]" \
                  "stack locals for same frame (level 1)"
- 
  }
  
  mi_runto callee4
--- 192,200 ----

Re: MI-related testsuite regressions

[Bob Rossi]

>  > >  Currently the default MI interpreter *is*
>  > > mi2, so these tests are just duplicated.  It would seem sensible, and save
>  > > effort, to have just one file (mi-stack.exp) until the MI version is bumped
>  > > up one number.  Then mi-stack.exp could be renamed mi2-stack.exp and a new
>  > > file mi-stack.exp could be created.
>  > > 
>  > > In reality GDB doesn't support more than one version of MI (the current
>  > > one).  As has been shown on the mailing list recently, even MI output
>  > > from GDB 6.3 differs from MI output from GDB in CVS.
>  > 
>  > mi2 is a released protocol; the intent is that we not make
>  > backwards-incompatible changes to mi2-*.exp, at least not without
>  > paying close attention to them.  -i=mi sets mi_version to 3 today. 
>  > Until we're ready to declare mi3 usable, we need to continue caring
>  > about mi2.
> 
> -i=mi sets mi_version to 2 in my copy:
> 
>   interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs));
> 
> and
> 
>   if (current_interp_named_p (INTERP_MI1))
>     deprecated_command_loop_hook = mi1_command_loop;
>   else if (current_interp_named_p (INTERP_MI2))
>     deprecated_command_loop_hook = mi2_command_loop;
>   else if (current_interp_named_p (INTERP_MI3))
>     deprecated_command_loop_hook = mi3_command_loop;
>   else
>     deprecated_command_loop_hook = mi2_command_loop;
> 
> 
> As far as I can see mi3 does nothing that mi2 doesn't do.

I think the ideology is that mi3 is allowed to change in such a way that
mi2 is not. So, all new features go into mi3, while mi2 should remain
stable and unchanged, except for bug fixes and minor new improvements
that are backwards compatible.

Bob Rossi

Re: MI-related testsuite regressions

[Daniel Jacobowitz]

On Mon, Jun 27, 2005 at 09:56:10AM -0400, Bob Rossi wrote:
> > As far as I can see mi3 does nothing that mi2 doesn't do.
> 
> I think the ideology is that mi3 is allowed to change in such a way that
> mi2 is not. So, all new features go into mi3, while mi2 should remain
> stable and unchanged, except for bug fixes and minor new improvements
> that are backwards compatible.

Yes.  I misread; what I should have said was "-i=mi3 exists today".

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: MI-related testsuite regressions

[Nick Roberts]

 > > -i=mi sets mi_version to 2 in my copy:
 > > 
 > >   interp_add (interp_new (INTERP_MI, NULL, mi_out_new (2), &procs));
 > > 
 > > and
 > > 
 > >   if (current_interp_named_p (INTERP_MI1))
 > >     deprecated_command_loop_hook = mi1_command_loop;
 > >   else if (current_interp_named_p (INTERP_MI2))
 > >     deprecated_command_loop_hook = mi2_command_loop;
 > >   else if (current_interp_named_p (INTERP_MI3))
 > >     deprecated_command_loop_hook = mi3_command_loop;
 > >   else
 > >     deprecated_command_loop_hook = mi2_command_loop;
 > > 
 > > 
 > > As far as I can see mi3 does nothing that mi2 doesn't do.
 > 
 > I think the ideology is that mi3 is allowed to change in such a way that
 > mi2 is not. So, all new features go into mi3, while mi2 should remain
 > stable and unchanged, except for bug fixes and minor new improvements
 > that are backwards compatible.

If thats the case, shouldn't the code read:

   interp_add (interp_new (INTERP_MI, NULL, mi_out_new (3), &procs));
 
 and
 
   if (current_interp_named_p (INTERP_MI1))
     deprecated_command_loop_hook = mi1_command_loop;
   else if (current_interp_named_p (INTERP_MI2))
     deprecated_command_loop_hook = mi2_command_loop;
   else if (current_interp_named_p (INTERP_MI3))
     deprecated_command_loop_hook = mi3_command_loop;
   else
     deprecated_command_loop_hook = mi3_command_loop;


as it stands the mi-*.exp don't test mi3.


Nick

Re: MI-related testsuite regressions

[Daniel Jacobowitz]

On Tue, Jun 28, 2005 at 02:08:22AM +1200, Nick Roberts wrote:
> If thats the case, shouldn't the code read:
> 
>    interp_add (interp_new (INTERP_MI, NULL, mi_out_new (3), &procs));
>  
>  and
>  
>    if (current_interp_named_p (INTERP_MI1))
>      deprecated_command_loop_hook = mi1_command_loop;
>    else if (current_interp_named_p (INTERP_MI2))
>      deprecated_command_loop_hook = mi2_command_loop;
>    else if (current_interp_named_p (INTERP_MI3))
>      deprecated_command_loop_hook = mi3_command_loop;
>    else
>      deprecated_command_loop_hook = mi3_command_loop;
> 
> 
> as it stands the mi-*.exp don't test mi3.

Probably, but I don't see much point in changing that until we have
some content for mi3.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: MI-related testsuite regressions

[Nick Roberts]

Daniel Jacobowitz writes:
 > On Tue, Jun 28, 2005 at 02:08:22AM +1200, Nick Roberts wrote:
 > > If thats the case, shouldn't the code read:
 > > 
 ...

 > > 
 > > as it stands the mi-*.exp don't test mi3.
 > 
 > Probably, but I don't see much point in changing that until we have
 > some content for mi3.


This could equally be said for having separate tests mi-*.exp and mi2-*.exp.

Nick

Re: MI-related testsuite regressions

[Daniel Jacobowitz]

On Tue, Jun 28, 2005 at 08:54:27AM +1200, Nick Roberts wrote:
> Daniel Jacobowitz writes:
>  > On Tue, Jun 28, 2005 at 02:08:22AM +1200, Nick Roberts wrote:
>  > > If thats the case, shouldn't the code read:
>  > > 
>  ...
> 
>  > > 
>  > > as it stands the mi-*.exp don't test mi3.
>  > 
>  > Probably, but I don't see much point in changing that until we have
>  > some content for mi3.
> 
> 
> This could equally be said for having separate tests mi-*.exp and mi2-*.exp.

It ensures that we don't develop unacceptable content in the existing
mi2 without noticing, as far as I'm concerned.

The existance of mi2-* maps to the fact that the protocol was released.

-- 
Daniel Jacobowitz
CodeSourcery, LLC