public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Tom de Vries <tdevries@suse.de>, gdb-patches@sourceware.org
Subject: Re: [PATCH 4/5] gdb/testsuite: change newline patterns used in gdb_test
Date: Tue, 02 May 2023 12:13:35 +0100	[thread overview]
Message-ID: <87ttwv7zq8.fsf@redhat.com> (raw)
In-Reply-To: <14333280-6e66-0cc5-7e34-46176588ee37@suse.de>

Tom de Vries <tdevries@suse.de> writes:

> On 5/1/23 16:33, Andrew Burgess wrote:
>> Tom de Vries <tdevries@suse.de> writes:
>> 
>>> On 3/31/23 22:20, Andrew Burgess via Gdb-patches wrote:
>>>> This commit makes two changes to how we match newline characters in
>>>> the gdb_test proc.
>>>
>>> Hi,
>>>
>>> the -wrap used in gdb_test_multiple is defined in terms of gdb_test
>>> semantics, but it doesn't seem to have been updated to match the new
>>> behaviour in gdb_test.
>>>
>>> I've filed a PR about this regression (
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=30403 ).
>> 
>> Sorry for any problems caused.  I'm not working today, but if this has
>> not been addressed, I'll look at this on Tuesday.
>
> AFAIU it's a silent regression, so there are no problems in term of 
> FAILs, it's just that more work is required.
>
> FWIW, I think the root cause for introducing this regression silently is 
> that we try to implement the same thing in two different locations, and 
> it's just easy for things to get out of sync.  I recently fixed 
> something similar in commit 4fa173cfd79 ("[gdb/testsuite] Fix -wrap in 
> presence of -prompt in gdb_test_multiple"), that's why I noticed it.

So I believe the patch below brings gdb_test_multiple with '-wrap' back
into line with gdb_test.  I also updated a couple of other places that
used the same (old) gdb_test pattern.

There were nowhere near as many regressions with this change as with
gdb_test.  Let me know what you think.

Thanks,
Andrew

---

commit 807658d7a9a32632554006e822ae8645cc4cabb6
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue May 2 10:56:55 2023 +0100

    gdb/testsuite: more newline pattern cleanup
    
    After this commit:
    
      commit e2f620135d92f7cd670af4e524fffec7ac307666
      Date:   Thu Mar 30 13:26:25 2023 +0100
    
          gdb/testsuite: change newline patterns used in gdb_test
    
    It was pointed out in PR gdb/30403 that the same patterns can be found
    in other lib/gdb.exp procs and that it would probably be a good idea
    if these procs remained in sync with gdb_test.  Actually, the bug
    specifically calls out gdb_test_multiple when using with '-wrap', but
    I found a couple of other locations in gdb_continue_to_breakpoint,
    gdb_test_multiline, get_valueof, and get_local_valueof.
    
    In all these locations one or both of the following issues are
    addressed:
    
      1. A leading pattern of '[\r\n]*' is pointless.  If there is a
      newline it will be matched, but if there is not then the testsuite
      doesn't care.  Also, as expect is happy to skip non-matched output
      at the start of a pattern, if there is a newline expect is happy to
      skip over it before matching the rest.  As such, this leading
      pattern is removed.
    
      2. Using '\[\r\n\]*$gdb_prompt' means that we will swallow
      unexpected blank lines at the end of a command's output, but also,
      if the pattern from the test script ends with a '\r', '\n', or '.'
      then these will partially match the trailing newline, with the
      remainder of the newline matched by the pattern from gdb.exp.  This
      split matching doesn't add any value, it's just something that has
      appeared as a consequence of how gdb.exp was originally written.  In
      this case the '\[\r\n\]*' is replaced with '\r\n'.
    
    I've rerun the testsuite and fixed the regressions that I saw, these
    were places where GDB emits a blank line at the end of the command
    output, which we now need to explicitly match in the test script, this
    was for:
    
      gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
      gdb.guile/guile.exp
      gdb.python/python.exp
    
    Or a location where the test script was matching part of the newline
    sequence, while gdb.exp was previously matching the remainder of the
    newline sequence.  Now we rely on gdb.exp to match the complete
    newline sequence, this was for:
    
      gdb.base/commands.exp
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30403

diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp
index ec2015ebef5..36918ed1a3f 100644
--- a/gdb/testsuite/gdb.base/commands.exp
+++ b/gdb/testsuite/gdb.base/commands.exp
@@ -725,7 +725,7 @@ maintenance deprecate set qqq_aaa"
 
 proc_with_prefix deprecated_command_alias_help_test {} {
     gdb_test_multiline "define real_command" \
-	"define real_command" "End with a line saying just \"end\".." \
+	"define real_command" "End with a line saying just \"end\"\\." \
 	"print 1" "" \
 	"end" ""
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
index bd3ea5b5d54..d2c28a87923 100644
--- a/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
+++ b/gdb/testsuite/gdb.dwarf2/dw2-out-of-range-end-of-seq.exp
@@ -88,7 +88,7 @@ gdb_test_multiple "maint info line-table $srcfile$" $test {
     -re -wrap "END *0x0*1 *$hex *Y *\r\n.*" {
 	fail $gdb_test_name
     }
-    -re -wrap "END *$hex *$hex *Y *" {
+    -re -wrap "END *$hex *$hex *Y *\r\n" {
 	pass $gdb_test_name
     }
 }
diff --git a/gdb/testsuite/gdb.guile/guile.exp b/gdb/testsuite/gdb.guile/guile.exp
index 7d0c063583d..ee8b2718178 100644
--- a/gdb/testsuite/gdb.guile/guile.exp
+++ b/gdb/testsuite/gdb.guile/guile.exp
@@ -63,7 +63,7 @@ gdb_test_multiline "show guile command" \
   "(print 23)" "" \
   "end" "" \
   "end" "" \
-  "show user zzq" "User command \"zzq\":.*  guile.*\\(print 23\\).*  end"
+  "show user zzq" "User command \"zzq\":.*  guile.*\\(print 23\\).*  end\r\n"
 
 gdb_test "source $host_source2_scm" "yes" "source source2.scm"
 
diff --git a/gdb/testsuite/gdb.python/python.exp b/gdb/testsuite/gdb.python/python.exp
index 7e9ddaa6fcd..584e52c0661 100644
--- a/gdb/testsuite/gdb.python/python.exp
+++ b/gdb/testsuite/gdb.python/python.exp
@@ -80,7 +80,7 @@ gdb_test_multiline "show python command" \
   "print (23)" "" \
   "end" "" \
   "end" "" \
-  "show user zzq" "User command \"zzq\":.*  python.*print \\(23\\).*  end"
+  "show user zzq" "User command \"zzq\":.*  python.*print \\(23\\).*  end\r\n"
 
 gdb_test_multiline "indented multi-line python command" \
   "python" "" \
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index aed7e2d043c..50c10333df1 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -818,7 +818,7 @@ proc gdb_continue_to_breakpoint {name {location_pattern .*}} {
 	-re "(?:Breakpoint|Temporary breakpoint) .* (at|in) $location_pattern\r\n$gdb_prompt $" {
 	    pass $full_name
 	}
-	-re "\[\r\n\]*(?:$kfail_pattern)\[\r\n\]+$gdb_prompt $" {
+	-re "(?:$kfail_pattern)\r\n$gdb_prompt $" {
 	    kfail "gdb/25038" $full_name
 	}
     }
@@ -1126,7 +1126,7 @@ proc gdb_test_multiple { command message args } {
 	if { $wrap_pattern } {
 	    # Wrap subst_item as is done for the gdb_test PATTERN argument.
 	    lappend $current_list \
-		"\[\r\n\]*(?:$subst_item)\[\r\n\]+$prompt_regexp"
+		"(?:$subst_item)\r\n$prompt_regexp"
 	    set wrap_pattern 0
 	} else {
 	    lappend $current_list $subst_item
@@ -1384,7 +1384,7 @@ proc gdb_test_multiline { name args } {
     foreach {input result} $args {
 	incr inputnr
 	if {[gdb_test_multiple $input "$name: input $inputnr: $input" {
-	    -re "\[\r\n\]*($result)\[\r\n\]+($gdb_prompt | *>)$" {
+	    -re "($result)\r\n($gdb_prompt | *>)$" {
 		pass $gdb_test_name
 	    }
 	}]} {
@@ -7746,7 +7746,7 @@ proc get_valueof { fmt exp default {test ""} } {
 
     set val ${default}
     gdb_test_multiple "print${fmt} ${exp}" "$test" {
-	-re "\\$\[0-9\]* = (\[^\r\n\]*)\[\r\n\]*$gdb_prompt $" {
+	-re "\\$\[0-9\]* = (\[^\r\n\]*)\r\n$gdb_prompt $" {
 	    set val $expect_out(1,string)
 	    pass "$test"
 	}
@@ -7770,7 +7770,7 @@ proc get_local_valueof { exp default {test ""} } {
 
     set val ${default}
     gdb_test_multiple "info locals ${exp}" "$test" {
-	-re "$exp = (\[^\r\n\]*)\[\r\n\]*$gdb_prompt $" {
+	-re "$exp = (\[^\r\n\]*)\r\n$gdb_prompt $" {
 	    set val $expect_out(1,string)
 	    pass "$test"
 	}


  reply	other threads:[~2023-05-02 11:13 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31 20:20 [PATCH 0/5] gdb/testsuite: stricter matching for gdb_test Andrew Burgess
2023-03-31 20:20 ` [PATCH 1/5] gdb/testsuite: fix occasional failure in gdb.base/clear_non_user_bp.exp Andrew Burgess
2023-03-31 20:20 ` [PATCH 2/5] gdb: remove some trailing newlines from warning messages Andrew Burgess
2023-03-31 20:20 ` [PATCH 3/5] gdb/testsuite: use 'return' in gdb_test_no_output Andrew Burgess
2023-03-31 20:20 ` [PATCH 4/5] gdb/testsuite: change newline patterns used in gdb_test Andrew Burgess
2023-04-27 19:39   ` Simon Marchi
2023-04-28 14:05     ` Andrew Burgess
2023-04-28 15:51     ` Andrew Burgess
2023-04-28 15:57       ` Simon Marchi
2023-04-28 18:37         ` Simon Marchi
2023-04-28 21:50           ` Andrew Burgess
2023-05-02 19:16             ` Simon Marchi
2023-04-29 15:20   ` Tom de Vries
2023-05-01 14:33     ` Andrew Burgess
2023-05-01 15:10       ` Tom de Vries
2023-05-02 11:13         ` Andrew Burgess [this message]
2023-05-02 14:48           ` Tom de Vries
2023-05-05 17:01             ` Andrew Burgess
2023-05-09  9:54               ` Andrew Burgess
2023-05-10  7:22                 ` Tom de Vries
2023-05-12 12:54                   ` Andrew Burgess
2023-03-31 20:20 ` [PATCH 5/5] gdb/testsuite: special case '^' in gdb_test pattern Andrew Burgess
2023-04-17 16:12 ` [PATCH 0/5] gdb/testsuite: stricter matching for gdb_test Tom Tromey
2023-04-27 12:58   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ttwv7zq8.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tdevries@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).