From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 887801F5AE for ; Fri, 23 Apr 2021 22:27:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237039AbhDWW2O (ORCPT ); Fri, 23 Apr 2021 18:28:14 -0400 Received: from mav.lukeshu.com ([104.207.138.63]:35802 "EHLO mav.lukeshu.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237181AbhDWW2L (ORCPT ); Fri, 23 Apr 2021 18:28:11 -0400 Received: from lukeshu-dw-thinkpad (unknown [IPv6:2601:281:8200:26:4e34:88ff:fe48:5521]) by mav.lukeshu.com (Postfix) with ESMTPSA id DD6EA80590; Fri, 23 Apr 2021 18:27:30 -0400 (EDT) Date: Fri, 23 Apr 2021 16:27:30 -0600 Message-ID: <87v98c1vsd.wl-lukeshu@lukeshu.com> From: Luke Shumaker To: Eric Sunshine Cc: Luke Shumaker , Git List , Avery Pennarun , Charles Bailey , Danny Lin , "David A . Greene" , David Aguilar , Jakub Suder , James Denholm , Jeff King , Jonathan Nieder , Junio C Hamano , =?UTF-8?B?Tmd1?= =?UTF-8?B?eeG7hW4g?= =?ISO-8859-1?Q?Th=E1i_?= =?UTF-8?B?Tmfhu41j?= Duy , Roger L Strain , Techlive Zheng , Luke Shumaker Subject: Re: [PATCH 11/30] subtree: t7900: add porcelain tests for 'pull' and 'push' In-Reply-To: References: <20210423194230.1388945-1-lukeshu@lukeshu.com> <20210423194230.1388945-12-lukeshu@lukeshu.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?ISO-8859-4?Q?Goj=F2?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Fri, 23 Apr 2021 14:19:28 -0600, Eric Sunshine wrote: > > On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker wrote: > > The 'pull' and 'push' subcommands deserve their own sections in the tests. > > Add some basic tests for them. > > > > Signed-off-by: Luke Shumaker > > --- > > diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh > > @@ -202,8 +202,8 @@ test_expect_success 'merge the added subproj again, should do nothing' ' > > test_expect_success 'merge new subproj history into subdir/ with a slash appended to the argument of --prefix' ' > > - test_create_repo "$test_count" && > > - test_create_repo "$test_count/subproj" && > > + subtree_test_create_repo "$test_count" && > > + subtree_test_create_repo "$test_count/subproj" && > > test_create_commit "$test_count" main1 && > > test_create_commit "$test_count/subproj" sub1 && > > This change doesn't seem to be related to the stated purpose of this > patch. Was it included by accident or is it just a drive-by "while at > it" fix that seems somewhat related since you're using > subtree_test_create_repo() in the newly-added tests? It might deserve > mention in the commit message. It was included by accident. I guess I'll move it to be in the "comment subtree_test_create_repo" commit, and mention it in the commit message there. > > @@ -427,6 +427,133 @@ test_expect_success 'split "sub dir"/ with --branch for an incompatible branch' > > +test_expect_success 'pull requires path given by option --prefix must exist' ' > > + test_create_commit "$test_count/sub proj" sub1 && > > + ( > > + test_must_fail git subtree pull --prefix="sub dir" ./"sub proj" HEAD >out 2>err && > > + > > + echo "'\''sub dir'\'' does not exist; use '\''git subtree add'\''" > expected && > > + test_must_be_empty out && > > + test_cmp expected err > > + ) > > +' > > The use of single-quotes and escaped single-quotes within the > single-quoted test body is breaking my brain. Perhaps take advantage > of SQ from test-lib.sh and interoplate it into the string rather than > dealing with raw single-quotes? > > echo "this $SQ is a single-quote" > > (After writing the above, I now see that you are just mirroring > existing practice in this test script. The single-quotes are > confusing, but following existing style may be important -- or not.) I don't think I'll change it, for consistency with the rest of the file... and I don't want to change the rest of the file, because there's enough churn already. But thanks for the tip, I wasn't aware of $SQ. > > +test_expect_success 'pull basic operation' ' > > + subtree_test_create_repo "$test_count" && > > + subtree_test_create_repo "$test_count/sub proj" && > > + test_create_commit "$test_count" main1 && > > + test_create_commit "$test_count/sub proj" sub1 && > > + ( > > + cd "$test_count" && > > + git fetch ./"sub proj" HEAD && > > I was going to comment on the unusual: > > ./"sub proj" > > rather than the more typical: > > "./sub proj" > > but I see that that also is mirroring existing practice in this > script, so... [intentionally left blank] ...yeah > > +test_expect_success 'push requires option --prefix' ' > > + subtree_test_create_repo "$test_count" && > > + subtree_test_create_repo "$test_count/sub proj" && > > + test_create_commit "$test_count" main1 && > > + test_create_commit "$test_count/sub proj" sub1 && > > + ( > > + cd "$test_count" && > > + git fetch ./"sub proj" HEAD && > > + git subtree add --prefix="sub dir" FETCH_HEAD && > > + echo "You must provide the --prefix option." > expected && > > + test_must_fail git subtree push "./sub proj" from-mainline > actual 2>&1 && > > Style: There is an inconsistent mix of "> foo" and ">foo" formatting > in the newly-added tests. These days, we prefer ">foo". Indeed, so do I :) That test was copied with minimal modification from the pre-existing 'split requires option --prefix' test, which wrote it that way. I guess I'll update the formatting commit to also normalize away the whitespace after ">"s. -- Happy hacking, ~ Luke Shumaker