Stefan Beller <sbeller@xxxxxxxxxx> writes:
> This adds tests for the atomic push option.
> The first four tests check if the atomic option works in
> good conditions and the last three patches check if the atomic
> option prevents any change to be pushed if just one ref cannot
> be updated.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
>
> Notes:
> Originally Ronnie had a similar patch prepared. But as I added
> more tests and cleaned up the existing tests (e.g. use test_commit
> instead of "echo one >file && gitadd file && git commit -a -m 'one'",
> removal of dead code), the file has changed so much that I'd rather
> take ownership.
>
> t/t5543-atomic-push.sh | 185 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 185 insertions(+)
> create mode 100755 t/t5543-atomic-push.sh
>
> diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
> new file mode 100755
> index 0000000..6cbedc5
> --- /dev/null
> +++ b/t/t5543-atomic-push.sh
> @@ -0,0 +1,185 @@
> +#!/bin/sh
> +
> +test_description='pushing to a repository using the atomic push option'
> +
> +. ./test-lib.sh
> +
> +D=`pwd`
$(pwd)???
> +mk_repo_pair () {
> + rm -rf workbench upstream thirdparty &&
> + mkdir upstream &&
> + (
> + cd upstream &&
> + git init --bare #&&
> + #git config receive.denyCurrentBranch warn
Please drop unused comments; they are distracting.
> + ) &&
> + mkdir workbench &&
> + (
> + cd workbench &&
> + git init &&
> + git remote add up ../upstream
> + )
> +}
Hmph. Shouldn't you be using test_create_repo to create these, so
that templates are used from the build (not install) location, and
their hooks are disabled?
> +test_push_failed () {
Does that mean "test_push_must_fail"?
> + workbench_master=$(cd workbench && git show-ref -s --verify refs/heads/master) &&
> + upstream_master=$(cd upstream && git show-ref -s --verify refs/heads/master) &&
> + test "$workbench_master" != "$upstream_master" &&
> +
> + workbench_second=$(cd workbench && git show-ref -s --verify refs/heads/second) &&
> + upstream_second=$(cd upstream && git show-ref -s --verify refs/heads/second) &&
> + test "$workbench_second" != "$upstream_second"
So the tests that use this helper always try to update master and
second? Is "they are different" the only thing that matters? Or
should you be verifying "They are left as the same value they used
to have before the attempted push that must have failed"?
It might be a good time to use "-C" option, e.g. "git -C workbench show-ref ...",
a bit more in our tests.
> +}
> +
> +test_push_succeeded () {
> + workbench_master=$(cd workbench && git show-ref -s --verify refs/heads/master) &&
> + upstream_master=$(cd upstream && git show-ref -s --verify refs/heads/master) &&
> + test "$workbench_master" = "$upstream_master"
Broken &&-chain?
> +
> + workbench_second=$(cd workbench && git show-ref -s --verify refs/heads/second) &&
> + upstream_second=$(cd upstream && git show-ref -s --verify refs/heads/second) &&
> + test "$workbench_second" = "$upstream_second"
> +}
> +
> +test_expect_success 'atomic push works for a single branch' '
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git push --mirror up &&
> + test_commit two &&
> + git push --atomic-push --mirror up
> + ) &&
> + workbench_master=$(cd workbench && git show-ref -s --verify refs/heads/master) &&
> + upstream_master=$(cd upstream && git show-ref -s --verify refs/heads/master) &&
> + test "$workbench_master" = "$upstream_master"
Hmph. It is a shame that the nice helper you created cannot be used
here.
> +'
> +
> +test_expect_success 'atomic push works for two branches' '
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git branch second &&
> + git push --mirror up &&
> + test_commit two &&
> + git checkout second &&
> + test_commit three &&
> + git push --atomic-push --mirror up
> + ) &&
> + test_push_succeeded
> +'
OK.
> +test_expect_success 'atomic push works in combination with --mirror' '
> + mk_repo_pair &&
> + ls workbench &&
Debugging?
> + (
> + cd workbench &&
> + test_commit one &&
> + git checkout -b second &&
> + test_commit two &&
> + git push --atomic-push --mirror up
It is not wrong per-se, but haven't you already tested in
combination with --mirror in the previous test?
> + ) &&
> + test_push_succeeded
> +'
> +
> +test_expect_success 'atomic push works in combination with --force' '
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git checkout -b second &&
> + test_commit two &&
> + test_commit three &&
> + test_commit four &&
> + git push --mirror up &&
> + git reset --hard HEAD^ &&
> + git push --force --atomic-push up master second
> + ) &&
> + test_push_succeeded
> +'
OK.
> +# set up two branches where master can be pushed but second can not
> +# (non-fast-forward). Since second can not be pushed the whole operation
> +# will fail and leave master untouched.
> +test_expect_success 'atomic push fails if one branch fails locally' '
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git checkout -b second master &&
> + test_commit two &&
> + test_commit three &&
> + test_commit four &&
> + git push --mirror up
Broken &&-chain.
> + git reset --hard HEAD~2 &&
> + git checkout master &&
> + test_commit five &&
> + ! git push --atomic-push --all up
test_must_fail?
> + ) &&
> + test_push_failed
You not only want to see master and second in upstream different from
those in workbench, but they point at specific commits that the previous
mirror push updated to.
Instead of test_push_failed / test_push_succeeded, how about a
single helper that takes the two commit object names you expect
these two branches point at? E.g.
check_branches upstream master HEAD@{2} second HEAD~
(I am probably miscounting HEAD@{$offset} here; this is for
illustration only) that roughly does
check_branches () {
target=$1
shift
while test $# -ne 0
do
git -C "$target" rev-parse --verify "refs/heads/$1" >actual &&
git rev-parse "$2" >expect &&
test_cmp expect actual || return 1
shift
shift
done
}
or something like that?
> +test_expect_success 'atomic push fails if one branch fails remotely' '
> + # prepare the repo
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git checkout -b second master &&
> + test_commit two &&
> + git push --mirror up
> + ) &&
> + # a third party modifies the server side:
> + (
> + git clone upstream thirdparty &&
> + cd thirdparty
> + git checkout second
> + test_commit unrelated_changes &&
> + git push origin second
> + ) &&
> + # see if we can now push both branches.
> + (
> + cd workbench &&
> + git checkout master &&
> + test_commit three &&
> + git checkout second &&
> + test_commit four &&
> + ! git push --atomic-push up master second
> + ) &&
> + test_push_failed
> +'
What's the value of this test? Isn't it a non-fast-forward check
you already tested in the previous one?
> +test_expect_success 'atomic push fails if one tag fails remotely' '
> + # prepare the repo
> + mk_repo_pair &&
> + (
> + cd workbench &&
> + test_commit one &&
> + git checkout -b second master &&
> + test_commit two &&
> + git push --mirror up
> + ) &&
> + # a third party modifies the server side:
> + (
> + git clone upstream thirdparty &&
> + cd thirdparty
> + git checkout second
> + git tag test_tag &&
> + git push --tags origin second
> + ) &&
Broken &&-chain aside, you do not need thirdparty. Doing the "git
tag" inside upstream itself should suffice.
> + # see if we can now push both branches.
> + (
> + cd workbench &&
> + git checkout master &&
> + test_commit three &&
> + git checkout second &&
> + test_commit four &&
> + git tag test_tag &&
> + ! git push --tags --atomic-push up master second
test_must_fail?
> + ) &&
> + test_push_failed
> +'
One more failure mode you would want to check is what if "update"
hook in the receiving repository rejected one update (but not the
other). Something along the lines of:
... setup ...
git -C workbench push up &&
write_script upstream/hooks/update <<-\EOF &&
# only allow update to master from now on
test "$1" = "refs/heads/master"
EOF
... further update to master and second ...
test_must_fail git -C workbench push up
check_branches upstream master ... second ...
perhaps?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html