This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]