| Message ID | 20210423194230.1388945-12-lukeshu@lukeshu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | subtree: clean up, improve UX | expand |
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".
On Fri, 23 Apr 2021 14:19:28 -0600, Eric Sunshine wrote: > > 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. 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.
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh index 6b655ab4b5..3ee0524233 100755 --- 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 && ( @@ -427,6 +427,133 @@ test_expect_success 'split "sub dir"/ with --branch for an incompatible branch' ) ' +# +# Tests for 'git subtree pull' +# + +test_expect_success 'pull 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 + ) && + test_create_commit "$test_count/sub proj" sub2 && + ( + cd "$test_count" && + test_must_fail git subtree pull ./"sub proj" HEAD >out 2>err && + + echo "You must provide the --prefix option." >expected && + test_must_be_empty out && + test_cmp expected err + ) +' + +test_expect_success 'pull requires path given by option --prefix must exist' ' + 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 && + ( + 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 + ) +' + +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 && + git subtree add --prefix="sub dir" FETCH_HEAD + ) && + test_create_commit "$test_count/sub proj" sub2 && + ( + cd "$test_count" && + exp=$(git -C "sub proj" rev-parse --verify HEAD:) && + git subtree pull --prefix="sub dir" ./"sub proj" HEAD && + act=$(git rev-parse --verify HEAD:"sub dir") && + test "$act" = "$exp" + ) +' + +# +# Tests for 'git subtree push' +# + +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 && + test_debug "printf '"expected: "'" && + test_debug "cat expected" && + test_debug "printf '"actual: "'" && + test_debug "cat actual" && + test_cmp expected actual + ) +' + +test_expect_success 'push requires path given by option --prefix must exist' ' + 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 "'\''non-existent-directory'\'' does not exist; use '\''git subtree add'\''" > expected && + test_must_fail git subtree push --prefix=non-existent-directory "./sub proj" from-mainline > actual 2>&1 && + test_debug "printf '"expected: "'" && + test_debug "cat expected" && + test_debug "printf '"actual: "'" && + test_debug "cat actual" && + test_cmp expected actual + ) +' + +test_expect_success 'push 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 && + git subtree add --prefix="sub dir" FETCH_HEAD + ) && + test_create_commit "$test_count" "sub dir"/main-sub1 && + test_create_commit "$test_count" main2 && + test_create_commit "$test_count/sub proj" sub2 && + test_create_commit "$test_count" "sub dir"/main-sub2 && + ( + cd "$test_count" && + git fetch ./"sub proj" HEAD && + git subtree merge --prefix="sub dir" FETCH_HEAD && + before=$(git rev-parse --verify HEAD) && + split_hash=$(git subtree split --prefix="sub dir") && + git subtree push --prefix="sub dir" ./"sub proj" from-mainline && + test "$before" = "$(git rev-parse --verify HEAD)" && + test "$split_hash" = "$(git -C "sub proj" rev-parse --verify refs/heads/from-mainline)" + ) +' + # # Validity checking #