[PATCH][gdb/testsuite] Introduce gdb_test_ext

[Tom de Vries]

On 19-09-19 18:18, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-09-19 13:13:23 +0200]:
> 
>> Hi,
>>
>> In commit 25e5c20918 "[gdb/testsuite] Allow some tests in gdb.base/store.exp
>> to be unsupported" we replace a gdb_test:
>> ...
>>     gdb_test "print l" " = ${l}" \
>>        "${prefix}; print old l, expecting ${l}"
>> ...
>> with a gdb_test_multiple:
>> ...
>>     set supported 1
>>     set test "${prefix}; print old l, expecting ${l}"
>>     gdb_test_multiple "print l" "$test"  {
>>        -re " = <optimized out>\r\n$gdb_prompt $" {
>>            unsupported $test
>>            set supported 0
>>        }
>>        -re " = ${l}\r\n$gdb_prompt $" {
>>            pass $test
>>        }
>>     }
>> ...
>> in order to handle the UNSUPPORTED case.
>>
>> This has the drawback that we have to be explicit about the gdb_prompt, and
>> move the gdb_test arguments around to fit the gdb_test_multiple format.
>>
>> Introduce a new proc gdb_test_ext that behaves as gdb_test, but also allows
>> extension, allowing us to rewrite the gdb_test_multiple above in a form
>> resembling the original gdb_test:
>> ...
>>     set supported 1
>>     gdb_test_ext "print l" " = ${l}" \
>>        "${prefix}; print old l, expecting ${l}" \
>>        -- [list "unsupported" " = <optimized out>" "set supported 0"]
> 
> I've also thought about this sort of problem in the past, and would
> like to propose a similar, but slightly different solution.
> 
> My idea is more like a trimmed down gdb_test_multiple, so for your
> example above you would write this:
> 
>     gdb_test_ext "print l" "${prefix}; print old l, expecting ${l}" {
> 	" = ${l}" {
> 	    pass $gdb_test_ext_name
> 	}
> 	" = <optimized out>" {
> 	    unsupported $gdb_test_ext_name
> 	    set supported 0
> 	}
>     }
> 
> You don't put '-re' before each pattern, this is because they aren't
> full patterns, gdb_test_ext will be extending them.
> 
> Unlike your solution the 'pass' case is not created automatically, you
> need to write it yourself, so maybe that's a negative.  The advantages
> I see of this approach is that there's not special handling for
> different "types" of alternatives as in your original code, the action
> block can contain anything 'unsupported', 'fail', etc.  Plus it's
> formatted as a code body, which I like.
> 

The solution as I proposed it doesn't limit itself to require each
alternative to be handled as either supported or pass or fail or
somesuch.  It just adds a means to extend gdb_test using a keyword that
determines how the keyword arguments are handled.

So, I've added the style you propose here as "generic", and rewrote one
of the two places I update in store.exp using the "generic" style for
demonstration purposes.

I envision the usage like this: you'd usually use "unsupported" or
similar to skip all the repetitive code and focus on the bits that
actually contain content, and for special cases where that doesn't fit
you'd use "generic". You can go further and add a "freeform" or some
such where you'd have to write out the entire regexp.

The nice thing is that you can add keywords and corresponding handling
as you go, whereas the gdb_test_multiple-like solution you propose only
has one way of handling its arguments, which of course does makes things
consistent and clear, but is not very extensible.

> One other thing which I've wanted for _ages_ is to avoid having to set
> the test name into a separate variable, which your solution offers
> too.  The solution I offer is '$gdb_test_ext_name', this variable is
> set auto-magically by the call to 'gdb_test_ext, and is available in
> all of the action bodies for calls to pass/fail/unsupported/etc.
> 

Nice trick, I've copied that for usage in the "generic" method.

So, WDYT?

Thanks,
- Tom

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-gdb-testsuite-Introduce-gdb_test_ext.patch
Type: text/x-patch
Size: 5942 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20190919/207e3be1/attachment.bin>