Recent MI patches broke the testsuite

Recent MI patches broke the testsuite

[Daniel Jacobowitz]

202-environment-directory ../../../src/gdb/testsuite/gdb.mi
202^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
(gdb) 
FAIL: gdb.mi/mi-basics.exp: environment-directory arg operation
203-environment-directory
203^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
(gdb) 
FAIL: gdb.mi/mi-basics.exp: environment-directory empty-string operation

It appears to be this patch:
2002-12-13  Jeff Johnston  <jjohnstn@redhat.com>

        * mi-cmds.c (-environment-directory) Change to use mi_cmd_env_dir,
        (-environment-cd): Change to use mi_cmd_env_cd,.
        (-environment-pwd): Change to use mi_cmd_env_pwd.
        (-environment-path): Change to use mi_cmd_env_path.
        * mi-cmds.h (mi_cmd_env_cd, mi_cmd_env_dir): New prototypes.
        (mi_cmd_env_path, mi_cmd_env_pwd): Ditto.
        * mi-cmd-env.c: New file.  Part of fix for PR gdb/741.
        * gdbmi.texinfo (environment-cd): Update output and example.
        (environment-pwd): Ditto.
        (environment-dir): Update output, description, and examples.
        (environment-path): Ditto.


Not only did it break several tests in gdb.mi/, it also broke the utility
function mi_gdb_reinitialize_dir in lib/mi-support.exp.  This causes all my
testsuite runs to time out.  Please don't commit patches like this without
testing them.

The output which expects "105-^done\r\n" needs to be updated as well as the
tests in mi-basics.exp; possibly elsewhere.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Recent MI patches broke the testsuite

[J. Johnston]

[-- Attachment #1: Type: text/plain, Size: 2222 bytes --]

My bad.  :(  The new mi-basics.exp testcase is my usual sanity test
for this change.  It was updated and in fact works fine as it is expecting
the new behavior.

I did not look at the correct gdb.log for the full mi testsuite.
As you have noted, the lib/mi-support.exp routine mi_reinitialize_dir
is using -environment-directory and is expecting the old output.

I have a patch for lib/mi=support.exp to handle the new syntax which I have
included.  Since I require approval for patches I will revert the
original patch and resubmit with the added change unless Elena says
otherwise.

-- Jeff J.

Daniel Jacobowitz wrote:
> 202-environment-directory ../../../src/gdb/testsuite/gdb.mi
> 202^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
> (gdb) 
> FAIL: gdb.mi/mi-basics.exp: environment-directory arg operation
> 203-environment-directory
> 203^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
> (gdb) 
> FAIL: gdb.mi/mi-basics.exp: environment-directory empty-string operation
> 
> It appears to be this patch:
> 2002-12-13  Jeff Johnston  <jjohnstn@redhat.com>
> 
>         * mi-cmds.c (-environment-directory) Change to use mi_cmd_env_dir,
>         (-environment-cd): Change to use mi_cmd_env_cd,.
>         (-environment-pwd): Change to use mi_cmd_env_pwd.
>         (-environment-path): Change to use mi_cmd_env_path.
>         * mi-cmds.h (mi_cmd_env_cd, mi_cmd_env_dir): New prototypes.
>         (mi_cmd_env_path, mi_cmd_env_pwd): Ditto.
>         * mi-cmd-env.c: New file.  Part of fix for PR gdb/741.
>         * gdbmi.texinfo (environment-cd): Update output and example.
>         (environment-pwd): Ditto.
>         (environment-dir): Update output, description, and examples.
>         (environment-path): Ditto.
> 
> 
> Not only did it break several tests in gdb.mi/, it also broke the utility
> function mi_gdb_reinitialize_dir in lib/mi-support.exp.  This causes all my
> testsuite runs to time out.  Please don't commit patches like this without
> testing them.
> 
> The output which expects "105-^done\r\n" needs to be updated as well as the
> tests in mi-basics.exp; possibly elsewhere.
> 


[-- Attachment #2: 741.lib.patch --]
[-- Type: text/plain, Size: 1603 bytes --]

Index: lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.19
diff -u -r1.19 mi-support.exp
--- lib/mi-support.exp	25 Nov 2002 19:54:09 -0000	1.19
+++ lib/mi-support.exp	16 Dec 2002 17:49:08 -0000
@@ -234,6 +234,7 @@
 
 proc mi_gdb_reinitialize_dir { subdir } {
     global mi_gdb_prompt
+    global MIFLAGS
 
     global suppress_flag
     if { $suppress_flag } {
@@ -244,8 +245,9 @@
 	return "";
     }
 
-    send_gdb "104-environment-directory\n"
-    gdb_expect 60 {
+    if { $MIFLAGS == "-i=mi1" } {
+      send_gdb "104-environment-directory\n"
+      gdb_expect 60 {
 	-re ".*Reinitialize source path to empty.*y or n. " {
             warning "Got confirmation prompt for dir reinitialization."
 	    send_gdb "y\n"
@@ -256,6 +258,14 @@
 	}
 	-re "$mi_gdb_prompt$" {}
         timeout {error "Dir reinitialization failed (timeout)"}
+      }
+    } else {
+      send_gdb "104-environment-directory -r\n"
+      gdb_expect 60 {
+	-re "104\\\^done,source-path=.*\r\n$mi_gdb_prompt$" {}
+	-re "$mi_gdb_prompt$" {}
+        timeout {error "Dir reinitialization failed (timeout)"}
+      }
     }
 
     send_gdb "105-environment-directory $subdir\n"
@@ -263,7 +273,7 @@
 	-re "Source directories searched.*$mi_gdb_prompt$" {
 	    verbose "Dir set to $subdir"
 	}
-	-re "105\\\^done\r\n$mi_gdb_prompt$" {
+	-re "105\\\^done.*\r\n$mi_gdb_prompt$" {
             # FIXME: We return just the prompt for now.
 	    verbose "Dir set to $subdir"
 	    # perror "Dir \"$subdir\" failed."

Re: Recent MI patches broke the testsuite

[Daniel Jacobowitz]

On Mon, Dec 16, 2002 at 01:04:42PM -0500, J. Johnston wrote:
> My bad.  :(  The new mi-basics.exp testcase is my usual sanity test
> for this change.  It was updated and in fact works fine as it is expecting
> the new behavior.

Not in my checkout:

202-environment-directory ../../../src/gdb/testsuite/gdb.mi
202^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
(gdb) 
FAIL: gdb.mi/mi-basics.exp: environment-directory arg operation

Oh, that's a different problem.  Using ${srcdir} that way doesn't work
if configured with a relative srcdir.  Just an extra wildcard will fix
it for now.

(i.e. I built this tree with ../../../src/configure)

> I did not look at the correct gdb.log for the full mi testsuite.
> As you have noted, the lib/mi-support.exp routine mi_reinitialize_dir
> is using -environment-directory and is expecting the old output.
> 
> I have a patch for lib/mi=support.exp to handle the new syntax which I have
> included.  Since I require approval for patches I will revert the
> original patch and resubmit with the added change unless Elena says
> otherwise.

Thank you very much for following up on this!  The testsuite patch
looks reasonable to my inexperienced eye.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Recent MI patches broke the testsuite

[Elena Zannoni]

J. Johnston writes:
 > My bad.  :(  The new mi-basics.exp testcase is my usual sanity test
 > for this change.  It was updated and in fact works fine as it is expecting
 > the new behavior.
 > 
 > I did not look at the correct gdb.log for the full mi testsuite.
 > As you have noted, the lib/mi-support.exp routine mi_reinitialize_dir
 > is using -environment-directory and is expecting the old output.
 > 
 > I have a patch for lib/mi=support.exp to handle the new syntax which I have
 > included.  Since I require approval for patches I will revert the
 > original patch and resubmit with the added change unless Elena says
 > otherwise.
 > 

If this patch fixes the testsuite, no need to revert the other one,
just check this in. Don't forget the ChangeLog.

Maybe Daniel can verify that it fixes his problem?

Elena


 > -- Jeff J.
 > 
 > Daniel Jacobowitz wrote:
 > > 202-environment-directory ../../../src/gdb/testsuite/gdb.mi
 > > 202^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
 > > (gdb) 
 > > FAIL: gdb.mi/mi-basics.exp: environment-directory arg operation
 > > 203-environment-directory
 > > 203^done,source-path="/opt/src/binutils/x86-as/gdb/testsuite/../../../src/gdb/testsuite/gdb.mi:$cdir:$cwd"
 > > (gdb) 
 > > FAIL: gdb.mi/mi-basics.exp: environment-directory empty-string operation
 > > 
 > > It appears to be this patch:
 > > 2002-12-13  Jeff Johnston  <jjohnstn@redhat.com>
 > > 
 > >         * mi-cmds.c (-environment-directory) Change to use mi_cmd_env_dir,
 > >         (-environment-cd): Change to use mi_cmd_env_cd,.
 > >         (-environment-pwd): Change to use mi_cmd_env_pwd.
 > >         (-environment-path): Change to use mi_cmd_env_path.
 > >         * mi-cmds.h (mi_cmd_env_cd, mi_cmd_env_dir): New prototypes.
 > >         (mi_cmd_env_path, mi_cmd_env_pwd): Ditto.
 > >         * mi-cmd-env.c: New file.  Part of fix for PR gdb/741.
 > >         * gdbmi.texinfo (environment-cd): Update output and example.
 > >         (environment-pwd): Ditto.
 > >         (environment-dir): Update output, description, and examples.
 > >         (environment-path): Ditto.
 > > 
 > > 
 > > Not only did it break several tests in gdb.mi/, it also broke the utility
 > > function mi_gdb_reinitialize_dir in lib/mi-support.exp.  This causes all my
 > > testsuite runs to time out.  Please don't commit patches like this without
 > > testing them.
 > > 
 > > The output which expects "105-^done\r\n" needs to be updated as well as the
 > > tests in mi-basics.exp; possibly elsewhere.
 > > 
 > 
 > Index: lib/mi-support.exp
 > ===================================================================
 > RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
 > retrieving revision 1.19
 > diff -u -r1.19 mi-support.exp
 > --- lib/mi-support.exp	25 Nov 2002 19:54:09 -0000	1.19
 > +++ lib/mi-support.exp	16 Dec 2002 17:49:08 -0000
 > @@ -234,6 +234,7 @@
 >  
 >  proc mi_gdb_reinitialize_dir { subdir } {
 >      global mi_gdb_prompt
 > +    global MIFLAGS
 >  
 >      global suppress_flag
 >      if { $suppress_flag } {
 > @@ -244,8 +245,9 @@
 >  	return "";
 >      }
 >  
 > -    send_gdb "104-environment-directory\n"
 > -    gdb_expect 60 {
 > +    if { $MIFLAGS == "-i=mi1" } {
 > +      send_gdb "104-environment-directory\n"
 > +      gdb_expect 60 {
 >  	-re ".*Reinitialize source path to empty.*y or n. " {
 >              warning "Got confirmation prompt for dir reinitialization."
 >  	    send_gdb "y\n"
 > @@ -256,6 +258,14 @@
 >  	}
 >  	-re "$mi_gdb_prompt$" {}
 >          timeout {error "Dir reinitialization failed (timeout)"}
 > +      }
 > +    } else {
 > +      send_gdb "104-environment-directory -r\n"
 > +      gdb_expect 60 {
 > +	-re "104\\\^done,source-path=.*\r\n$mi_gdb_prompt$" {}
 > +	-re "$mi_gdb_prompt$" {}
 > +        timeout {error "Dir reinitialization failed (timeout)"}
 > +      }
 >      }
 >  
 >      send_gdb "105-environment-directory $subdir\n"
 > @@ -263,7 +273,7 @@
 >  	-re "Source directories searched.*$mi_gdb_prompt$" {
 >  	    verbose "Dir set to $subdir"
 >  	}
 > -	-re "105\\\^done\r\n$mi_gdb_prompt$" {
 > +	-re "105\\\^done.*\r\n$mi_gdb_prompt$" {
 >              # FIXME: We return just the prompt for now.
 >  	    verbose "Dir set to $subdir"
 >  	    # perror "Dir \"$subdir\" failed."

Re: Recent MI patches broke the testsuite

[Daniel Jacobowitz]

On Mon, Dec 16, 2002 at 01:24:28PM -0500, Elena Zannoni wrote:
> J. Johnston writes:
>  > My bad.  :(  The new mi-basics.exp testcase is my usual sanity test
>  > for this change.  It was updated and in fact works fine as it is expecting
>  > the new behavior.
>  > 
>  > I did not look at the correct gdb.log for the full mi testsuite.
>  > As you have noted, the lib/mi-support.exp routine mi_reinitialize_dir
>  > is using -environment-directory and is expecting the old output.
>  > 
>  > I have a patch for lib/mi=support.exp to handle the new syntax which I have
>  > included.  Since I require approval for patches I will revert the
>  > original patch and resubmit with the added change unless Elena says
>  > otherwise.
>  > 
> 
> If this patch fixes the testsuite, no need to revert the other one,
> just check this in. Don't forget the ChangeLog.
> 
> Maybe Daniel can verify that it fixes his problem?

Yes, it does, except for the mi-basics.exp part.  And that's minor.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: Recent MI patches broke the testsuite

[Jason Molenda]

On Sun, Dec 15, 2002 at 04:11:32PM -0500, Daniel Jacobowitz wrote:

> Not only did it break several tests in gdb.mi/, it also broke the utility
> function mi_gdb_reinitialize_dir in lib/mi-support.exp.  This causes all my
> testsuite runs to time out.  Please don't commit patches like this without
> testing them.


Remarkably enough, these two patches from over a month ago:


Fixing MI frame tuple output when there is a call dummy or signal frame
on the stack, includes test cases for new output:
	http://sources.redhat.com/ml/gdb-patches/2002-11/msg00274.html

Fixing var-list-children's response so CHILDREN is a list instead of
a tuple, includes updates to testsuite and doc fix (already approved
by Eli):
	http://sources.redhat.com/ml/gdb-patches/2002-11/msg00287.html


Do not break the testsuite.  They also include a brand new test
case for the corrected output, updates to the testsuite in the
other, and documentation fixes where needed.  Remarkable specimens
of patchy goodness, if I may say so myself.

And yet (cue violins) they languish...

Jason

Re: Recent MI patches broke the testsuite

[J. Johnston]

Daniel Jacobowitz wrote:
> On Mon, Dec 16, 2002 at 01:24:28PM -0500, Elena Zannoni wrote:
> 
>>J. Johnston writes:
>> > My bad.  :(  The new mi-basics.exp testcase is my usual sanity test
>> > for this change.  It was updated and in fact works fine as it is expecting
>> > the new behavior.
>> > 
>> > I did not look at the correct gdb.log for the full mi testsuite.
>> > As you have noted, the lib/mi-support.exp routine mi_reinitialize_dir
>> > is using -environment-directory and is expecting the old output.
>> > 
>> > I have a patch for lib/mi=support.exp to handle the new syntax which I have
>> > included.  Since I require approval for patches I will revert the
>> > original patch and resubmit with the added change unless Elena says
>> > otherwise.
>> > 
>>
>>If this patch fixes the testsuite, no need to revert the other one,
>>just check this in. Don't forget the ChangeLog.
>>
>>Maybe Daniel can verify that it fixes his problem?
> 
> 
> Yes, it does, except for the mi-basics.exp part.  And that's minor.
> 

Great.  I have checked in the patch from the previous note.  I will
look into making the mi-basics.exp test more robust with regards
to relative paths without overly abusing .*.

-- Jeff J.