[PATCH 1/2] [gdb/testsuite] Detect trailing-text-in-parentheses duplicates

[Andrew Burgess]

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

> When using a duplicate test name:
> ...
> fail foo
> fail foo
> ...
> we get:
> ...
> FAIL: $exp: foo
> FAIL: $exp: foo
> DUPLICATE: $exp: foo
> ...
>
> But when we do:
> ...
> fail foo
> fail "foo (timeout)"
> ...
> we get only:
> ...
> FAIL: $exp: foo
> FAIL: $exp: foo (timeout)
> ...
>
> Trailing text between parentheses prefixed with a space is interpreted as
> extra information, and not as part of the test name [1].
>
> Consequently, "foo" and "foo (timeout)" do count as duplicate test names,
> which should have been detected.  This is PR testsuite/29772.

Thanks for fixing this.  I had one question, see below...

>
> Fix this in CheckTestNames::_check_duplicates, such that we get:
> ...
> FAIL: $exp: foo
> FAIL: $exp: foo (timeout)
> DUPLICATE: $exp: foo (timeout)
> ...
>
> [ One note on the implementation: I used the regexp { \([^()]*\)$}. I don't
> know whether that covers all required cases, due to the fact that those are
> not unambiguousely specified.  It might be possible to reverse-engineer that
> information by reading or running the "regression analysis tools" mentioned on
> the wiki page [1], but I haven't been able to.  Regardless, the current regexp
> covers a large amount of cases, which IMO should be sufficient to be
> acceptable. ]
>
> Doing so shows many new duplicates in the testsuite.
>
> A significant number of those is due to using a message which is a copy of the
> command:
> ...
> gdb_test "print (1)"
> ...
>
> Fix this by handling those cases using test names "gdb-command<print (1)>" and
> "gdb-command<print (2)>.
>
> Fix the remaining duplicates manually (split off as follow-up patch for
> readability of this patch).
>
> Tested on x86_64-linux and aarch64-linux.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29772
>
> [1] https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
> ---
>  gdb/testsuite/lib/check-test-names.exp | 10 ++++++++
>  gdb/testsuite/lib/gdb.exp              | 34 ++++++++++++++++++++++----
>  2 files changed, 39 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/testsuite/lib/check-test-names.exp b/gdb/testsuite/lib/check-test-names.exp
> index 8f86f31aad4..11aed63ed11 100644
> --- a/gdb/testsuite/lib/check-test-names.exp
> +++ b/gdb/testsuite/lib/check-test-names.exp
> @@ -64,6 +64,16 @@ namespace eval ::CheckTestNames {
>      proc _check_duplicates { message } {
>  	variable all_test_names
>  
> +	# Remove test-case prefix, including the space separator.
> +	set prefix [string_to_regexp "$::subdir/$::gdb_test_file_name.exp: "]
> +	set message [regsub ^$prefix $message ""]
> +
> +	# Remove the "extra information" part.
> +	set message [regsub { \([^()]*\)$} $message ""]
> +
> +	# Add back the test-case prefix.
> +	set message "${prefix}$message"

Maybe I missed it, but it's not clear to me why you're removing the
filename prefix and then adding the regexp version of the filename back
on again.  Could you explain this part bit more please.

Thanks,
Andrew



> +
>  	# Initialise a count, or increment the count for this test name.
>  	if {![info exists all_test_names($message)]} {
>  	    set all_test_names($message) 0
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index dfe19c9410d..e1ca9a6459f 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -965,6 +965,31 @@ proc fill_in_default_prompt {prompt_regexp with_anchor} {
>      return $prompt_regexp
>  }
>  
> +# Generate message from COMMAND.
> +#
> +# This is not trivial in the case that the command contains parentheses.
> +# Trailing text between parentheses prefixed with a space is interpreted as
> +# extra information, and not as part of the test name [1].  Consequently,
> +# "PASS: print (1)" and "PASS: print (2)" count as duplicates.
> +#
> +# We fix this here by using "PASS: gdb-command<print (1)>" and
> +# "PASS: gdb-command<print (2)>".
> +#
> +# A trivial way to fix this in a test-case is by using gdb_test "print(1)",
> +# which produces the nicer-looking "PASS: print(1)".
> +#
> +# [1] https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages
> +
> +proc command_to_message { command } {
> +    set message $command
> +
> +    if { [regexp { \(([^()]*)\)$} $message] } {
> +	set message gdb-command<$message>
> +    }
> +
> +    return $message
> +}
> +
>  # gdb_test_multiple COMMAND MESSAGE [ -prompt PROMPT_REGEXP] [ -lbl ]
>  #                   EXPECT_ARGUMENTS
>  # Send a command to gdb; test the result.
> @@ -1095,7 +1120,7 @@ proc gdb_test_multiple { command message args } {
>      set prompt_regexp [fill_in_default_prompt $prompt_regexp true]
>  
>      if { $message == "" } {
> -	set message $command
> +	set message [command_to_message $command]
>      }
>  
>      if [string match "*\[\r\n\]" $command] {
> @@ -1471,7 +1496,6 @@ proc gdb_test_multiline { name args } {
>      return 0
>  }
>  
> -
>  # gdb_test [-prompt PROMPT_REGEXP] [-lbl]
>  #          COMMAND [PATTERN] [MESSAGE] [QUESTION RESPONSE]
>  # Send a command to gdb; test the result.
> @@ -1529,7 +1553,7 @@ proc gdb_test { args } {
>      }
>  
>      if { $message == "" } {
> -	set message $command
> +	set message [command_to_message $command]
>      }
>  
>      set prompt [fill_in_default_prompt $prompt [expr !${no-prompt-anchor}]]
> @@ -1795,7 +1819,7 @@ proc gdb_test_lines { command message re args } {
>      }
>  
>      if { $message == ""} {
> -	set message $command
> +	set message [command_to_message $command]
>      }
>  
>      set lines ""
> @@ -2012,7 +2036,7 @@ proc gdb_test_stdio {command inferior_pattern {gdb_pattern ""} {message ""}} {
>      global gdb_prompt
>  
>      if {$message == ""} {
> -	set message $command
> +	set message [command_to_message $command]
>      }
>  
>      set inferior_matched 0
>
> base-commit: a81f4e591fdb8530d832addc39beb31353b0ef2d
> -- 
> 2.35.3