Re: [PATCH] Prevent turning record on while threads are running (PR 20869)

[Luis Machado <lgustavo@codesourcery.com>]

On 11/29/2016 10:42 AM, Simon Marchi wrote:
> On 16-11-29 10:58 AM, Luis Machado wrote:
>>> +if ![supports_reverse] {
>>
>> Add an explicit untested call here?
>
> Right, adding:
>
> untested "reverse debugging not supported"
>
>>> +proc test_record_while_running { } {
>>> +    gdb_test "continue &" "Continuing."
>>> +    gdb_test "record" "Can't enable record while the program is running.  Use \"interrupt\" to stop it first."
>>
>> I have mixed feelings with the above test names. I'd know what to look
>> for in case of failure, but more explicit test names wouldn't hurt for a
>> quick inspection of the logs.
>>
>> "move thread"
>> "switch record on when thread is moving"
>>
>> Feel free to pick it up though. Not a hard requirement.
>
> You are right, it helps when reading the test.  The command by itself doesn't
> convey why we are using doing that command.  How about:
>
>   proc_with_prefix test_record_while_running { } {
>       gdb_test "continue &" "Continuing." "resume target"
>       gdb_test \
>           "record" \
>           "Can't enable record while the program is running.  Use \"interrupt\" to stop it first." \
>           "switch record on while target is running"
>   }
>
> PASS: gdb.reverse/record-while-running.exp: test_record_while_running: resume target
> PASS: gdb.reverse/record-while-running.exp: test_record_while_running: switch record on while target is running
>
> I added proc_with_prefix, I think it can help by giving some context to the messages.
>
> Thanks for the feedback,
>
> Simon
>

The above looks good to me.

Thanks,
Luis