From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20519 invoked by alias); 5 Mar 2013 19:02:44 -0000 Received: (qmail 20507 invoked by uid 22791); 5 Mar 2013 19:02:42 -0000 X-SWARE-Spam-Status: No, hits=-8.0 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,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; Tue, 05 Mar 2013 19:02:31 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r25J2TaX015917 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 5 Mar 2013 14:02:29 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r25J2Sva006298; Tue, 5 Mar 2013 14:02:28 -0500 Message-ID: <51364143.30302@redhat.com> Date: Tue, 05 Mar 2013 19:02:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 MIME-Version: 1.0 To: Yao Qi CC: gdb-patches@sourceware.org Subject: tstatus.exp: use UNSUPPORTED for optional features that are not supported (was: Re: [PATCH] Fix a bug in tstatus.exp matching tstatus output) References: <1362411345-22308-1-git-send-email-yao@codesourcery.com> <5134CC91.804@redhat.com> <51355AC1.1080000@codesourcery.com> In-Reply-To: <51355AC1.1080000@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: 2013-03/txt/msg00162.txt.bz2 On 03/05/2013 02:38 AM, Yao Qi wrote: > On 03/05/2013 12:32 AM, Pedro Alves wrote: >> On 03/04/2013 03:35 PM, Yao Qi wrote: >>> (gdb) PASS: gdb.trace/tstatus.exp: tstatus does not report trace stop reason >> ... >>> (gdb) PASS: gdb.trace/tstatus.exp: tstatus reports trace stop reason >> >> I think the former (and any other similar case) should be >> UNSUPPORTED rather than a PASS. That'd make it much easier >> to spot these issues, along with regressions and progressions. >> > Several lines above, the comments say: > > # Now play with tstatus a bit. > # Since note support is optional, we need to match both with and without > # cases. > > so all tests below here match two cases (with and without notes). > I am not sure we can change it to UNSUPPORTED, but I think we can. More so since the "not supported" paths actually explicitly check for output you'd get if the feature wasn't supported, so real unexpected failures should be caught as FAILs. So now e.g., we wanted to check if tstatus reports the trace stop reason, and if the target does, we get PASS: tstatus reports trace stop reason if the target reports what we'd expect if trace stop reason isn't supported, we get: UNSUPPORTED: tstatus reports trace stop reason and if the target reports something else unexpected, we get: FAIL: tstatus reports trace stop reason That has the advantage that the test string is always the same and only the test results change (PASS/FAIL/UNSUPPORTED), which makes it easier for testers see regressions, compared to: -PASS: gdb.trace/tstatus.exp: tstatus reports trace stop reason +PASS: gdb.trace/tstatus.exp: tstatus does not report trace stop reason which clearly easily goes by unnoticed, as evidenced by the existing problem your patch addresses. > it is not the purpose or motivation of this patch. It can be > addressed separately. Yes, of course. Here's a patch to address it. This is what I get with current tree (_without_ your patch): Running target native-gdbserver Running ../../../src/gdb/testsuite/gdb.trace/tstatus.exp ... PASS: gdb.trace/tstatus.exp: IPA loaded PASS: gdb.trace/tstatus.exp: tracepoint at gdb_c_test PASS: gdb.trace/tstatus.exp: collect at set_point: define actions PASS: gdb.trace/tstatus.exp: 4-byte fast tracepoint could not be set PASS: gdb.trace/tstatus.exp: advance to trace begin PASS: gdb.trace/tstatus.exp: start trace experiment PASS: gdb.trace/tstatus.exp: advance through tracing PASS: gdb.trace/tstatus.exp: tstatus reports trace note PASS: gdb.trace/tstatus.exp: change tracing note PASS: gdb.trace/tstatus.exp: tstatus reports different trace note PASS: gdb.trace/tstatus.exp: change tracing user PASS: gdb.trace/tstatus.exp: tstatus reports trace user PASS: gdb.trace/tstatus.exp: trace stopped with note UNSUPPORTED: gdb.trace/tstatus.exp: tstatus reports trace stop reason PASS: gdb.trace/tstatus.exp: info trace reports tracepoint hit count and traceframe usage gdb/testsuite/ 2013-03-05 Pedro Alves * gdb.trace/tstatus.exp (run_trace_experiment): When the target doesn't support the tested optional feature, call "unsupported" with the same test message as the "pass" case, instead of calling "pass" with a different message. Use the same text for the "fail" cases too. --- gdb/testsuite/gdb.trace/tstatus.exp | 36 ++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/gdb/testsuite/gdb.trace/tstatus.exp b/gdb/testsuite/gdb.trace/tstatus.exp index 8a0bbdf..d95a1c5 100644 --- a/gdb/testsuite/gdb.trace/tstatus.exp +++ b/gdb/testsuite/gdb.trace/tstatus.exp @@ -73,57 +73,59 @@ proc run_trace_experiment {} { # Since note support is optional, we need to match both with and without # cases. + set test "tstatus reports trace note" gdb_test_multiple "tstatus" "check on trace status" { -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Trace notes: my tracing note\.\[\r\n\]+Not looking at any trace frame\..*\r\n$gdb_prompt $" { - pass "tstatus reports trace note" + pass $test } -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Not looking at any trace frame.*\r\n$gdb_prompt $" { - pass "tstatus does not report any trace note" + unsupported $test } } gdb_test "set trace-notes different note" "" "change tracing note" - gdb_test_multiple "tstatus" "check on trace status with diff note" { + set test "tstatus reports different trace note" + gdb_test_multiple "tstatus" $test { -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Trace notes: different note\.\[\r\n\]+Not looking at any trace frame\..*\r\n$gdb_prompt $" { - pass "tstatus reports different trace note" + pass $test } -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Not looking at any trace frame.*\r\n$gdb_prompt $" { - pass "tstatus does not report any different trace note" + unsupported $test } } gdb_test "set trace-user me me me" "" "change tracing user" - gdb_test_multiple "tstatus" "check on trace status with diff note" { + set test "tstatus reports trace user" + gdb_test_multiple "tstatus" $test { -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Trace user is me me me\.\[\r\n\]+Trace notes: different note\.\[\r\n\]+Not looking at any trace frame\..*\r\n$gdb_prompt $" { - pass "tstatus reports trace user" + pass $test } -re "Trace is running.*Trace will stop if GDB disconnects\.\[\r\n\]+Not looking at any trace frame.*\r\n$gdb_prompt $" { - pass "tstatus does not report trace user" + unsupported $test } } gdb_test_no_output "tstop because I can" "trace stopped with note" - gdb_test_multiple "tstatus" "check on trace status after stop" { + set test "tstatus reports trace stop reason" + gdb_test_multiple "tstatus" $test { -re "Trace stopped by a tstop command (because I can)\..*Trace will stop if GDB disconnects\.\[\r\n\]+Trace user is me me me\.\[\r\n\]+Trace notes: different note\.\[\r\n\]+Not looking at any trace frame\..*\r\n$gdb_prompt $" { - pass "tstatus reports trace stop reason" + pass $test } -re "Trace stopped by a tstop command\..*\r\n$gdb_prompt $" { - pass "tstatus does not report trace stop reason" + unsupported $test } } - # Hit count and traceframe usage of tracepoint is optional, so - # pass it either way. - - gdb_test_multiple "info trace" "show tracepoint state" { + set test "info trace reports tracepoint hit count and traceframe usage" + gdb_test_multiple "info trace" $test { -re "actions\.c:\[0-9\]+\[\r\n\]+\[\t ]+tracepoint already hit 1 time\[\r\n\]+\[\t ]+trace buffer usage ${decimal} bytes\.\[\r\n\]+\[\t ]+collect parm.*\r\n$gdb_prompt $" { - pass "info trace reports tracepoint hit count and traceframe usage" + pass $test } -re "actions\.c:\[0-9\]+\[\r\n\]+\[\t ]+collect parm.*\r\n$gdb_prompt $" { - pass "info trace does not report tracepoint hit count and traceframe usage" + unsupported $test } } }