Re: [RFC PATCH v4 26/26] test smart http fetch and push

[Tay Ray Chuan <rctay89@gmail.com>]

Hi,

On Thu, Oct 29, 2009 at 8:00 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> The top level directory "/git/" of the test Apache server is mapped
> through our git-http-backend CGI, but uses the same underlying
> repository space as the server's document root.  This is the most
> simple installation possible.

Having "/git/" reside as a subdirectory in "/" where WebDAV is enabled
may be confusing to readers. I think we should use "/smart/" for the
CGI map, and consequently, use "/dumb/" for WebDAV repositories,
rather than the root "/" that it is occupying.

> diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
>[snip]
> +test_expect_success 'push to remote repository with packed refs' '
> +       cd "$ROOT_PATH"/test_repo_clone &&
> +       : >path2 &&
> +       git add path2 &&
> +       test_tick &&
> +       git commit -m path2 &&
> +       HEAD=$(git rev-parse --verify HEAD) &&
> +       git push &&
> +       (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
> +        test $HEAD = $(git rev-parse --verify HEAD))
> +'
> +
> +test_expect_success 'push already up-to-date' '
> +       git push
> +'
> +
> +test_expect_success 'push to remote repository with unpacked refs' '
> +       (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
> +        rm packed-refs &&
> +        git update-ref refs/heads/master $ORIG_HEAD) &&
> +       git push &&
> +       (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git &&
> +        test $HEAD = $(git rev-parse --verify HEAD))
> +'

Mention of "packed refs" should be removed from the description, and
the 'unpacked refs' test, irrelevant in this context, should be
removed too. The assumptions these tests are based on is relevant to
t5540, but not in t5541. My explanation follows.

(Clemens, the following also addresses your non-desire to remove the
"unpacked refs" test in your earlier email
<20091025161630.GB8532@localhost>.)

In the "old" (v1.6.5) http push mechanism, no refspec is passed to the
http-push helper. This shouldn't be the case, because match_refs is
done in transport.c::transport_push, but then transport->push_refs
isn't defined so this doesn't happen.

http-push is then depended on to learn of the remote refs and match
them itself, but it does badly at this, since it only recurses through
/refs/heads in the remote repository. None could be found, since
they've all been packed. Thus the push in the first test failed.
Nothing has been pushed to the remote repository. The following push
in the "unpacked refs" corrects this after "unpacking" the refs.

Clearly, these test are about the ability of http push mechanism to
learn of refs in /refs/heads and (lack thereof) from /packed-refs.

But in this patch series, this is no longer the case.
transport->push_refs is now defined, so what happens is that the
http-push helper is passed a refspec, unlike in the "old" mechanism.
http-push, using this refspec, now matches refs properly (though it
still does its recursion thing). Thus the push in the first test
(should) succeed. The push in the "unpacked refs" test is now
irrelevant, since the first push has already successfully pushed
changes to the remote repository.

So, what we should have is just one no-frills push test, keeping as
well the "push already up-to-date" test.

-- 
Cheers,
Ray Chuan