From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 70039 invoked by alias); 29 Nov 2016 16:47:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 70010 invoked by uid 89); 29 Nov 2016 16:47:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=Hx-languages-length:1601 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Nov 2016 16:47:21 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cBlYu-0004q0-7f from Luis_Gustavo@mentor.com ; Tue, 29 Nov 2016 08:47:20 -0800 Received: from [172.30.5.15] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Tue, 29 Nov 2016 08:47:17 -0800 Reply-To: Luis Machado Subject: Re: [PATCH] Prevent turning record on while threads are running (PR 20869) References: <20161129150758.29912-1-simon.marchi@ericsson.com> <8b198908-9a78-d2e8-1726-a471d3afb9b7@ericsson.com> To: Simon Marchi , CC: From: Luis Machado Message-ID: Date: Tue, 29 Nov 2016 16:47:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <8b198908-9a78-d2e8-1726-a471d3afb9b7@ericsson.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-04.mgc.mentorg.com (147.34.90.204) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00962.txt.bz2 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