Re: [PATCH] Make tests expect [ \t]+ pattern instead of \t for "info reg" command

[Ruslan Kabatsayev <b7.10110111@gmail.com>]

On 8 January 2018 at 07:25, Joel Brobecker <brobecker@adacore.com> wrote:
> On Mon, Jan 08, 2018 at 01:18:30AM +0300, Ruslan Kabatsayev wrote:
>> This will allow to format output of "info reg" command as we wish,
>> without breaking the tests. In particular, it'll let us correctly align
>> raw and natural values of the registers using spaces instead of current
>> badly-working approach with tabs.
>>
>> This change is forwards- and backwards-compatible, so that the amended
>> tests will work in the same way before and after reformatting patches
>> (unless the tests check formatting, of course, but I've not come across
>> any such tests).
>>
>> Some tests already used this expected pattern, so they didn't
>> even have to be modified. Others are changed by this patch.
>>
>> I've checked this on a i386 system, with no noticeable differences in
>> test results, so at least on i386 nothing seems to be broken by this.
>>
>> gdb/testsuite/ChangeLog:
>>
>>       * gdb.arch/powerpc-d128-regs.exp: Replace expected "\[\t\]*" from
>>       "info reg" with "\[ \t\]*".
>>       * gdb.arch/altivec-regs.exp: Replace expected "\t" from "info reg" with
>>       "\[ \t\]+".
>>       * gdb.arch/s390-multiarch.exp: Ditto.
>>       * gdb.base/pc-fp.exp: Ditto.
>>       * gdb.reverse/i386-precsave.exp: Ditto.
>>       * gdb.reverse/i386-reverse.exp: Ditto.
>>       * gdb.reverse/i387-env-reverse.exp: Ditto.
>>       * gdb.reverse/i387-stack-reverse.exp: Ditto.
>
> No objection on my end. I might have used \\s instead of \[ \t\],
> but perhaps there was a reason not to; and even if not, no point
> in redoing it all now.

\\s would also allow newlines, form feeds etc., which doesn't look
like a good idea here.

>
> Anyone against this change?
>
> One comment:
>
>> diff --git a/gdb/testsuite/gdb.base/pc-fp.exp b/gdb/testsuite/gdb.base/pc-fp.exp
>> index 5230b2b..8a81db4 100644
>> --- a/gdb/testsuite/gdb.base/pc-fp.exp
>> +++ b/gdb/testsuite/gdb.base/pc-fp.exp
>> @@ -58,4 +58,4 @@ gdb_test "info register \$fp" "${valueof_fp}.*"
>>  # Regression test for
>>  # http://sourceware.org/bugzilla/show_bug.cgi?id=12659
>>  gdb_test "info register pc fp" \
>> -    "pc +${valueof_pc}\t${valueof_pc} <.*>\[\r\n\]+fp +${valueof_fp}\t${valueof_fp}\[\r\n\]+"
>> +    "pc +${valueof_pc}\[ \t\]+${valueof_pc} <.*>\[\r\n\]+fp +${valueof_fp}\[ \t\]+${valueof_fp}\[\r\n\]+"
>
> This change looks like it's doing more than just allowing spaces
> on top of tabs...

Well it allows _multiple_ spaces/tabs instead of exactly one tab —
exactly what we need to be compatible with space-alignment. In other
respects I don't see what else it does.

>
> For the rest of this (large and mechanical patch), I tried my best
> to pay attention to all the details.
>
> --
> Joel