From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23011 invoked by alias); 23 May 2010 19:38:55 -0000 Received: (qmail 23002 invoked by uid 22791); 23 May 2010 19:38:54 -0000 X-SWARE-Spam-Status: No, hits=-2.1 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-outbound-2.vmware.com (HELO smtp-outbound-2.vmware.com) (65.115.85.73) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 23 May 2010 19:38:50 +0000 Received: from mailhost4.vmware.com (mailhost4.vmware.com [10.16.67.124]) by smtp-outbound-2.vmware.com (Postfix) with ESMTP id 5B8E23A007; Sun, 23 May 2010 12:38:49 -0700 (PDT) Received: from msnyder-server.eng.vmware.com (promd-2s-dhcp138.eng.vmware.com [10.20.124.138]) by mailhost4.vmware.com (Postfix) with ESMTP id 4FBC7C9A5F; Sun, 23 May 2010 12:38:49 -0700 (PDT) Message-ID: <4BF98448.5030805@vmware.com> Date: Sun, 23 May 2010 20:15:00 -0000 From: Michael Snyder User-Agent: Thunderbird 2.0.0.22 (X11/20090609) MIME-Version: 1.0 To: Pedro Alves CC: "gdb-patches@sourceware.org" Subject: Re: [RFA] gdb.base/c*.exp, send_gdb vs. gdb_test References: <4BF84A62.40806@vmware.com> <201005222335.58505.pedro@codesourcery.com> In-Reply-To: <201005222335.58505.pedro@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2010-05/txt/msg00540.txt.bz2 Pedro Alves wrote: > On Saturday 22 May 2010 22:19:30, Michael Snyder wrote: > >> Index: call-ar-st.exp >> =================================================================== >> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/call-ar-st.exp,v >> retrieving revision 1.24 >> diff -u -p -r1.24 call-ar-st.exp >> --- call-ar-st.exp 5 May 2010 18:06:57 -0000 1.24 >> +++ call-ar-st.exp 22 May 2010 21:15:10 -0000 >> @@ -61,24 +61,19 @@ set timeout [expr "$timeout + 60"] >> proc set_lang_c {} { >> global gdb_prompt >> >> - send_gdb "set language c\n" >> - gdb_expect { >> - -re ".*$gdb_prompt $" {} >> - timeout { fail "set language c (timeout)" ; return 0; } >> - } >> + gdb_test_no_output "set language c" >> >> - send_gdb "show language\n" >> - gdb_expect { >> + gdb_test_multiple "show language" "set language to \"c\"" { >> -re ".* source language is \"c\".*$gdb_prompt $" { >> pass "set language to \"c\"" >> return 1 >> } >> -re ".*$gdb_prompt $" { >> - fail "setting language to \"c\"" >> + fail "set language to \"c\"" >> return 0 >> } >> timeout { >> - fail "can't show language (timeout)" >> + fail "(timeout) set language to \"c\"" >> return 0 >> } > > You'll need to have the pass branch set a variable, and > do the return outside gdb_test_multiple. This is because > gdb_test_multiple catches other error reasons (the whole purpose > of gdb_test_multiple). This pattern shows up in several > other files, I'll skip mentioning those from here on. OK, I'll fix it in this instance. The other instances are never called, so I'll just remove them. > > (I'm not sure whether you've missed the "return 0" on the > "set" test above: it does seem okay to let the "show" test > handle it.) > >> #call compute_with_small_structs(20) >> -send_gdb "print compute_with_small_structs(20)\n" >> -gdb_expect { >> - -re ".*\[0-9\]+ =.*$gdb_prompt $" { >> - pass "print compute_with_small_structs(20)" >> - } >> - -re ".*$gdb_prompt $" { fail "print compute_with_small_structs(20)" } >> - timeout { fail "(timeout) compute_with_small_structs(20)" } >> - } >> +gdb_test "print compute_with_small_structs(20)" \ >> + "\[0-9\]+ = void" \ >> + "print compute_with_small_structs(20)" >> > > You didn't resist changing the expected output here? :-) Caught me. > >> - send_gdb "step\n" >> - gdb_expect { >> - -re " >> -init_bit_flags_combo \\(bit_flags_combo=, a=1, b=0, ch1=121 .y., g=1, d=0, ch2=110 .n., e=1, o=0\\) at .*call-ar-st.c:416\[ \t\n\r\]+416.*bit_flags_combo->alpha = a;.*$gdb_prompt $" { >> - pass "step into init_bit_flags_combo"} >> - -re ".*$gdb_prompt $" { fail "step into init_bit_flags_combo" } >> - timeout { fail "step into init_bit_flags_combo (timeout)" } >> - } >> +gdb_test "step" \ >> + "init_bit_flags_combo \\(bit_flags_combo=, a=1, b=0, ch1=121 .y., g=1, d=0, ch2=110 .n., e=1, o=0\\) at .*call-ar-st.c:416\[ \t\n\r\]+416.*bit_flags_combo->alpha = a;" \ >> + "step into init_bit_flags_combo" >> > > Missing .* at end of expected output? I saw several of > those; I don't know if this was on purpose. > > On Saturday 22 May 2010 22:19:30, Michael Snyder wrote: >> + gdb_test_multiple "bt" "backtrace" { >> + -re "#(\[0-9\]*) *.*$gdb_prompt $" { >> + return $expect_out(1,string) >> + } >> + -re "$gdb_prompt $" { >> + return "" >> + } >> + timeout { >> + return "" >> + } >> + } >> + return "" > > Similar remark one of the above, but in this case, there's already a > return outside gdb_test_multiple, so you can remove the last -re and > the timeout handling. OK. >> if ![gdb_skip_stdio_test "call str_func1(s)"] { >> - send_gdb "call str_func1(s)\n" >> - gdb_expect { >> - -re "first string arg is: test string.*\"test string\".*$gdb_prompt $" { >> - pass "call str_func1(s)" >> - } >> - -re ".*$gdb_prompt $" { fail "call str_func1(s)" } >> - timeout { fail "(timeout) call str_func1(s)" } >> - } >> + gdb_test "call str_func1(s)" \ > > The previous test output didn't have the extra space. Could you > remove it? There are several instances of this. What extra space are you talking about? > > I've been assuming you've been diffing gdb.sum before/after > these patches; it'd be nice if you mentioned any changes shown by > that diff. I run the test after each change, and note any changes in numbers of pass/fails. > >> + "first string arg is: test string.*\"test string\".*" >> } > > Are you planning on committing the patches that have been > reviewed soon? I think it would be a good idea to get them in > sooner than later, so that we can see if there are any problems > we may have missed in review, and so we'd avoid propagating the > same mistakes in all these other patches. >