From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 54182383F87A for ; Tue, 26 May 2020 15:35:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 54182383F87A Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-440-jmIXfwpHPi-hr-tFV8L87Q-1; Tue, 26 May 2020 11:35:23 -0400 X-MC-Unique: jmIXfwpHPi-hr-tFV8L87Q-1 Received: by mail-wr1-f71.google.com with SMTP id w4so4315543wrl.13 for ; Tue, 26 May 2020 08:35:23 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=g9buW8NlxybD5h8N+gsvYyMzMiDNcWNdu8tJnSmDv0E=; b=me6kMJ8Zf9IV30Qiq1rH//B80c/LiCBHWxIe56BRecE7rLgB2fE24siyFqJnliY1a9 BQ9v/j++gTb4yl5WjkCB51IDJlKNkjbrDyj3JPPOkayMLyFV4QDhZNSsESLGio/wXeAZ iqJkor0jbXv2Ml9XrFWIvhHAHLHL3d7AJPT/7cM9vtI3makVTKnoeUooVB2oAWyniZ95 /vsYbRyAlnnT5wtmvN/JafLlCuXONk0UFoKRBSaPYze4/ttcjCqfo9YnHGXWr9Ka8Ivg UtRq8Rn/B9yN/a2J/KdHXz52gZwKDiE6XWhUlvBuGfUET81we6AonAMA9YpIIIfwQ7yN Bh2A== X-Gm-Message-State: AOAM533whgjECYTgd7C2QnjFkH6/0aa7dgvTesNFZ3TI8FsSqKpwNiY/ rJa1qrvgMRwAWMEPkadRTp983//PnaV++CXiBKrPSYRCDw94JBEP0fp7hADy+i4zk7Vd9fHAs5I rON/R4YdSIVWaqWfdrk7sGg== X-Received: by 2002:a7b:ce01:: with SMTP id m1mr1950771wmc.116.1590507322404; Tue, 26 May 2020 08:35:22 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzthGk/zqS2hRH5yN1YMIPKvIK69pzsL/fsNX2go+5VinThroP8lonM7Uw9nUi0El6Ap5f+Rw== X-Received: by 2002:a7b:ce01:: with SMTP id m1mr1950752wmc.116.1590507322154; Tue, 26 May 2020 08:35:22 -0700 (PDT) Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id p4sm215879wrq.31.2020.05.26.08.35.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 May 2020 08:35:21 -0700 (PDT) Subject: Re: [PATCH, testsuite] Fix some duplicate test names To: Luis Machado , gdb-patches@sourceware.org References: <20200526141848.22771-1-luis.machado@linaro.org> From: Pedro Alves Message-ID: <5aa1d0a1-f674-efc8-bcb5-c71e36363e65@redhat.com> Date: Tue, 26 May 2020 16:35:20 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200526141848.22771-1-luis.machado@linaro.org> Content-Language: en-US X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 May 2020 15:35:30 -0000 Thanks! Some comments as I quickly skimmed the patch. Some of the issues appear more than once, but I didn't point at all instances. On 5/26/20 3:18 PM, Luis Machado via Gdb-patches wrote: > # Test that GDB manages caches correctly for tagged address. > # Read from P2, > -gdb_test "x p2" "$hex:\[\t \]+0x000004d2" > +gdb_test "x p2" "$hex:\[\t \]+0x000004d2" "x p2" > gdb_test_no_output "set variable i = 5678" > # Test that *P2 is updated. > -gdb_test "x p2" "$hex:\[\t \]+0x0000162e" > +gdb_test "x p2" "$hex:\[\t \]+0x0000162e" "x p2 (updated)" > NAK here -- these should be considered the same. See: https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Do_not_use_.22tail_parentheses.22_on_test_messages If the detection machinery does not consider them duplicates, then it should be fixed. > gdb_test_multiple "break *test_ldr_literal_16" "break test_ldr_literal_16" { > -re "Breakpoint.*at.* file .*$srcfile, line.*\r\n$gdb_prompt $" { > - pass "break test_ldr_literal" > + pass "break test_ldr_literal_16" > } Why not "break *break test_ldr_literal_16"? I.e., the "*" is missing. But consider $gdb_test_name, and dropping the explicit test name in the gdb_test_multiple call too: gdb_test_multiple "break *test_ldr_literal_16" "" { -re "Breakpoint.*at.* file .*$srcfile, line.*\r\n$gdb_prompt $" { pass $gdb_test_name } > gdb_test_multiple "break *test_zero_cbnz" "break test_zero_cbnz" { > -re "Breakpoint.*at.* file .*$srcfile, line.*\r\n$gdb_prompt $" { > - pass "break test_ldr_literal" > + pass "break test_ldr_literal in test_zero_cbnz" > } This one shows why $gdb_test_name would be better -- here a fail caught by gdb_test_multiple's internal patterns will issue a different message compared to a PASS: FAIL: break test_zero_cbnz PASS: break test_ldr_literal in test_zero_cbnz > -gdb_test "print x" "$decimal = 45" > +gdb_test "print x" "$decimal = 45" "validate setting a globa, 2nd time" > Typo: "globa". > +++ b/gdb/testsuite/gdb.base/return2.exp > @@ -50,7 +50,7 @@ proc return_1 { type } { > gdb_test "print ${type}_resultval == testval.${type}_testval" ".* = 1" \ > "${type} value returned successfully" > gdb_test "print ${type}_resultval != ${type}_returnval" ".* = 1" \ > - "validate result value not equal to program return value" > + "validate result value not equal to program return value (${type})" > } That issue with tail parens again. > +gdb_test "p foo1_3 (a)" "Cannot resolve.*" \ > + "pointer to pointer of wrong type (a)" > +gdb_test "p foo1_3 (bp)" "Cannot resolve.*" \ > + "pointer to pointer of wrong type (bp)" > gdb_test "p foo1_4 (bp)" "= 14" "pointer to ancestor pointer" Ditto. > +with_test_prefix "Strict type checking on" { > + gdb_test "p foo1_type_check (123)" [format $error_str "foo1_type_check"] > + gdb_test "p foo2_type_check (0, 1)" [format $error_str "foo2_type_check"] Lowercase "Strict". > @@ -401,25 +401,25 @@ void do_frozen_tests () > v1.nested.k = 9; > /*: > set_frozen V1 1 > - mi_varobj_update * {} "update varobjs: nothing changed" > - mi_check_varobj_value V1.i 1 "check V1.i: 1" > - mi_check_varobj_value V1.nested.j 2 "check V1.nested.j: 2" > - mi_check_varobj_value V1.nested.k 3 "check V1.nested.k: 3" > + mi_varobj_update * {} "update varobjs: nothing changed, 2nd" > + mi_check_varobj_value V1.i 1 "check V1.i: 1, 2nd" > + mi_check_varobj_value V1.nested.j 2 "check V1.nested.j: 2, 2nd" > + mi_check_varobj_value V1.nested.k 3 "check V1.nested.k: 3, 2nd" > # Check that explicit update for elements of structures > # works. > # Update v1.j > mi_varobj_update V1.nested.j {V1.nested.j} "update V1.nested.j" > - mi_check_varobj_value V1.i 1 "check V1.i: 1" > - mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8" > - mi_check_varobj_value V1.nested.k 3 "check V1.nested.k: 3" > + mi_check_varobj_value V1.i 1 "check V1.i: 1, 3rd" > + mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8, 1st" > + mi_check_varobj_value V1.nested.k 3 "check V1.nested.k: 3, 3rd" > # Update v1.nested, check that children is updated. > mi_varobj_update V1.nested {V1.nested.k} "update V1.nested" > mi_check_varobj_value V1.i 1 "check V1.i: 1" > - mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8" > + mi_check_varobj_value V1.nested.j 8 "check V1.nested.j: 8, 2nd" > mi_check_varobj_value V1.nested.k 9 "check V1.nested.k: 9" > # Update v1.i > mi_varobj_update V1.i {V1.i} "update V1.i" > - mi_check_varobj_value V1.i 7 "check V1.i: 7" > + mi_check_varobj_value V1.i 7 "check V1.i: 7, 1st" Here, instead of the counting, it would seem to me better to use with_test_prefix, like with_test_prefix "update *" { ... } with_test_prefix "update v1.j" { ... } with_test_prefix "v1.nested" { ... } Thanks, Pedro Alves