From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13022 invoked by alias); 12 Dec 2012 12:24:03 -0000 Received: (qmail 12991 invoked by uid 22791); 12 Dec 2012 12:24:00 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 12 Dec 2012 12:23:52 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBCCNov9027164 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 12 Dec 2012 07:23:50 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qBCCNniI017672; Wed, 12 Dec 2012 07:23:49 -0500 Message-ID: <50C87754.7010807@redhat.com> Date: Wed, 12 Dec 2012 12:24:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: Re: [PATCH 5/6] Test tracepoints are installed or not References: <1354596282-32526-1-git-send-email-yao@codesourcery.com> <1354596282-32526-6-git-send-email-yao@codesourcery.com> <50C1E38F.2020108@redhat.com> <50C1F553.8030502@codesourcery.com> <50C1FB3A.8050802@redhat.com> <50C489B3.8050101@codesourcery.com> <50C7731A.7040007@gmail.com> <50C7F30C.1050906@codesourcery.com> In-Reply-To: <50C7F30C.1050906@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2012-12/txt/msg00383.txt.bz2 On 12/12/2012 02:59 AM, Yao Qi wrote: > On 12/12/2012 01:53 AM, Pedro Alves wrote: >> >> this use of pass+exp_continue is fragile. Say a GDB bug is >> introduced, and the second =breakpoint-modified fails to be output. In >> that case, this will result in: >> >> PASS: tracepoint on pendfunc2 resolved >> FAIL: tracepoint on pendfunc2 resolved >> >> instead of >> PASS: tracepoint on pendfunc2 resolved >> FAIL: tracepoint on pendfunc2 installed >> > > My intention of using exp_coutine here is to handle the situation that > notifications may arrive in different orders. The fail message is not > clear, but not a big deal, to me. It is important to catch a fail and > then we can check gdb.log and test case to fix it. The text of the fail message isn't that important, I agree, but ending up with the _same_ text for both the PASS and the FAIL (the whole duplicate messages issue, PR13443) is something we're actively trying to avoid. > >> You could fix that by doing: >> >> + # Pending tracepoint is resolved. >> pass "$test" >> + set test "tracepoint on pendfunc2 installed" >> + exp_continue >> >> But then, if the order of the notifications changes (IOW, due to a bug, >> we end up with the tracepoint not installed), this won't catch it. > > Looks the fix is worse than the original one, at least no fail is > missing in the original one. Not sure what you mean. The only change compared to the original one would be the new set test "tracepoint on pendfunc2 installed" line. Sorry if that wasn't clear. (you'd probably change the other pass to be 'pass "$test"' too then.) > >> It seems best to me to only use exp_continue in cases we won't to >> consume/skip output, and in the case of this patch, split the two >> tests into two consecutive gdb_expects. > > If this way, we can't handle that two notifications arrive in a > reversed order (which is also correct). This seems to be crux of the issue here. I don't understand how reverse order would be correct. This: =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n" =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y" means the frontend ends up thinking the tracepoint is installed, while this: =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="y" =breakpoint-modified,bkpt={number="1",type="tracepoint".*.*times=\"0\".*installed="n" means the frontend ends up thinking the tracepoint is not installed, which I'd think is not what we want in this test. > If we don't have to worry about the order Confused, because I'm suggesting to worry about the order. :-) > (after all, the order of MI notifications are still > deterministic nowadays), I am OK with your suggestion. > 2012-12-12 Yao Qi > > * gdb.trace/mi-tracepoint-changed.exp (test_pending_resolved): Check > 'installed' field in '=breakpoint-modified'. > (test_reconnect): Check 'installed' field in > '=breakpoint-modified' and '=breakpoint-created'. This version looks good to me. Thanks, -- Pedro Alves