Re: [PATCH 11/30] subtree: t7900: add porcelain tests for 'pull' and 'push'

> 
> On Fri, Apr 23, 2021 at 3:43 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:
> > The 'pull' and 'push' subcommands deserve their own sections in the tests.
> > Add some basic tests for them.
> >
> > Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
> > ---
> > 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.

> > @@ -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.)

> > +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]

> > +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".