Push tests now work against Github repos

6 files changed, 56 insertions, 81 deletions

diff --git a/src/push.c b/src/push.c
index 353006b37..424959e8c 100644
--- a/src/push.c
+++ b/src/push.c
@@ -374,13 +374,9 @@ int git_push_finish(git_push *push)
 
 	if ((error = filter_refs(push->remote)) < 0 ||
 		(error = do_push(push)) < 0)
-		goto on_error;
-
-	error = 0;
+		return error;
 
-on_error:
-	git_remote_disconnect(push->remote);
-	return error;
+	return 0;
 }
 
 int git_push_unpack_ok(git_push *push)
diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c
index cb88f5058..99578f462 100644
--- a/src/transports/smart_protocol.c
+++ b/src/transports/smart_protocol.c
@@ -487,7 +487,8 @@ on_success:
 	error = 0;
 
 on_error:
-	writepack->free(writepack);
+	if (writepack)
+		writepack->free(writepack);
 
 	/* Trailing execution of progress_cb, if necessary */
 	if (npp.callback && npp.stats->received_bytes > npp.last_fired_bytes)
@@ -687,6 +688,13 @@ int git_smart__push(git_transport *transport, git_push *push)
 	else if (parse_report(&t->buffer, push) < 0)
 		goto on_error;
 
+	/* If we updated at least one ref, then we need to re-acquire the list of 
+	 * refs so the caller can call git_remote_update_tips afterward. TODO: Use
+	 * the data from the push report to do this without another network call */
+	if (push->specs.length &&
+		t->parent.connect(&t->parent, t->url, t->cred_acquire_cb, GIT_DIR_PUSH, t->flags) < 0)
+		goto on_error;
+
 	error = 0;
 
 on_error:
diff --git a/tests-clar/network/push.c b/tests-clar/network/push.c
index 883542836..adfe0aa86 100644
--- a/tests-clar/network/push.c
+++ b/tests-clar/network/push.c
@@ -17,7 +17,7 @@ static git_remote *_remote;
 static record_callbacks_data _record_cbs_data = {{ 0 }};
 static git_remote_callbacks _record_cbs = RECORD_CALLBACKS_INIT(&_record_cbs_data);
 
-static git_oid _oid_master;
+static git_oid _oid_b6;
 static git_oid _oid_b5;
 static git_oid _oid_b4;
 static git_oid _oid_b3;
@@ -109,7 +109,7 @@ static void do_verify_push_status(git_push *push, const push_status expected[],
 
 /**
  * Verifies that after git_push_finish(), refs on a remote have the expected
- * names, oids, and order, then disconnects.
+ * names, oids, and order.
  * 
  * @param remote remote to verify
  * @param expected_refs expected remote refs after push
@@ -119,12 +119,6 @@ static void verify_refs(git_remote *remote, expected_ref expected_refs[], size_t
 {
 	git_vector actual_refs = GIT_VECTOR_INIT;
 
-	/* git_push_finish() disconnects the remote. */
-	cl_assert(!git_remote_connected(remote));
-
-	/* Reconnect to the remote to verify the refs */
-	cl_git_pass(git_remote_connect(remote, GIT_DIR_PUSH));
-
 	git_remote_ls(remote, record_ref_cb, &actual_refs);
 	verify_remote_refs(&actual_refs, expected_refs, expected_refs_len);
 
@@ -145,7 +139,7 @@ void test_network_push__initialize(void)
 	rewrite_gitmodules(git_repository_workdir(_repo));
 
 	/* git log --format=oneline --decorate --graph
-	 * *-.   951bbbb90e2259a4c8950db78946784fb53fcbce (HEAD, master) merge b3, b4, and b5 to master
+	 * *-.   951bbbb90e2259a4c8950db78946784fb53fcbce (HEAD, b6) merge b3, b4, and b5 to b6
 	 * |\ \
 	 * | | * fa38b91f199934685819bea316186d8b008c52a2 (b5) added submodule named 'submodule' pointing to '../testrepo.git'
 	 * | * | 27b7ce66243eb1403862d05f958c002312df173d (b4) edited fold\b.txt
@@ -155,7 +149,7 @@ void test_network_push__initialize(void)
 	 * * a78705c3b2725f931d3ee05348d83cc26700f247 (b2, b1) added fold and fold/b.txt
 	 * * 5c0bb3d1b9449d1cc69d7519fd05166f01840915 added a.txt
 	 */
-	git_oid_fromstr(&_oid_master, "951bbbb90e2259a4c8950db78946784fb53fcbce");
+	git_oid_fromstr(&_oid_b6, "951bbbb90e2259a4c8950db78946784fb53fcbce");
 	git_oid_fromstr(&_oid_b5, "fa38b91f199934685819bea316186d8b008c52a2");
 	git_oid_fromstr(&_oid_b4, "27b7ce66243eb1403862d05f958c002312df173d");
 	git_oid_fromstr(&_oid_b3, "d9b63a88223d8367516f50bd131a5f7349b7f3e4");
@@ -196,11 +190,16 @@ void test_network_push__initialize(void)
 
 			cl_git_pass(git_push_finish(push));
 			git_push_free(push);
-			verify_refs(_remote, NULL, 0);
 		}
 
 		git_remote_disconnect(_remote);
 		git_vector_free(&delete_specs);
+
+		/* Now that we've deleted everything, fetch from the remote */
+		cl_git_pass(git_remote_connect(_remote, GIT_DIR_FETCH));
+		cl_git_pass(git_remote_download(_remote, NULL, NULL));
+		cl_git_pass(git_remote_update_tips(_remote));
+		git_remote_disconnect(_remote);
 	} else
 		printf("GITTEST_REMOTE_URL unset; skipping push test\n");
 }
@@ -270,17 +269,6 @@ void test_network_push__noop(void)
 	do_push(NULL, 0, NULL, 0, NULL, 0, 0);
 }
 
-void test_network_push__master(void)
-{
-	const char *specs[] = { "refs/heads/master:refs/heads/master" };
-	push_status exp_stats[] = { { "refs/heads/master", NULL } };
-	expected_ref exp_refs[] = { { "refs/heads/master", &_oid_master } };
-
-	do_push(specs, ARRAY_SIZE(specs),
-		exp_stats, ARRAY_SIZE(exp_stats),
-		exp_refs, ARRAY_SIZE(exp_refs), 0);
-}
-
 void test_network_push__b1(void)
 {
 	const char *specs[] = { "refs/heads/b1:refs/heads/b1" };
@@ -338,24 +326,21 @@ void test_network_push__multi(void)
 		"refs/heads/b2:refs/heads/b2",
 		"refs/heads/b3:refs/heads/b3",
 		"refs/heads/b4:refs/heads/b4",
-		"refs/heads/b5:refs/heads/b5",
-		"refs/heads/master:refs/heads/master"
+		"refs/heads/b5:refs/heads/b5"
 	};
 	push_status exp_stats[] = {
 		{ "refs/heads/b1", NULL },
 		{ "refs/heads/b2", NULL },
 		{ "refs/heads/b3", NULL },
 		{ "refs/heads/b4", NULL },
-		{ "refs/heads/b5", NULL },
-		{ "refs/heads/master", NULL }
+		{ "refs/heads/b5", NULL }
 	};
 	expected_ref exp_refs[] = {
 		{ "refs/heads/b1", &_oid_b1 },
 		{ "refs/heads/b2", &_oid_b2 },
 		{ "refs/heads/b3", &_oid_b3 },
 		{ "refs/heads/b4", &_oid_b4 },
-		{ "refs/heads/b5", &_oid_b5 },
-		{ "refs/heads/master", &_oid_master }
+		{ "refs/heads/b5", &_oid_b5 }
 	};
 	do_push(specs, ARRAY_SIZE(specs),
 		exp_stats, ARRAY_SIZE(exp_stats),
@@ -385,20 +370,20 @@ void test_network_push__implicit_tgt(void)
 
 void test_network_push__fast_fwd(void)
 {
-	/* Fast forward b1 in tgt from _oid_b1 to _oid_master. */
+	/* Fast forward b1 in tgt from _oid_b1 to _oid_b6. */
 
 	const char *specs_init[] = { "refs/heads/b1:refs/heads/b1" };
 	push_status exp_stats_init[] = { { "refs/heads/b1", NULL } };
 	expected_ref exp_refs_init[] = { { "refs/heads/b1", &_oid_b1 } };
 
-	const char *specs_ff[] = { "refs/heads/master:refs/heads/b1" };
+	const char *specs_ff[] = { "refs/heads/b6:refs/heads/b1" };
 	push_status exp_stats_ff[] = { { "refs/heads/b1", NULL } };
-	expected_ref exp_refs_ff[] = { { "refs/heads/b1", &_oid_master } };
+	expected_ref exp_refs_ff[] = { { "refs/heads/b1", &_oid_b6 } };
 
 	/* Do a force push to reset b1 in target back to _oid_b1 */
 	const char *specs_reset[] = { "+refs/heads/b1:refs/heads/b1" };
 	/* Force should have no effect on a fast forward push */
-	const char *specs_ff_force[] = { "+refs/heads/master:refs/heads/b1" };
+	const char *specs_ff_force[] = { "+refs/heads/b6:refs/heads/b1" };
 
 	do_push(specs_init, ARRAY_SIZE(specs_init),
 		exp_stats_init, ARRAY_SIZE(exp_stats_init),
@@ -508,7 +493,7 @@ void test_network_push__bad_refspecs(void)
 		cl_git_pass(git_push_new(&push, _remote));
 
 		/* Unexpanded branch names not supported */
-		cl_git_fail(git_push_add_refspec(push, "master:master"));
+		cl_git_fail(git_push_add_refspec(push, "b6:b6"));
 
 		git_push_free(push);
 	}
@@ -530,32 +515,4 @@ void test_network_push__expressions(void)
 	do_push(specs_right_expr, ARRAY_SIZE(specs_right_expr),
 		exp_stats_right_expr, ARRAY_SIZE(exp_stats_right_expr),
 		NULL, 0, 0);
-}
-
-void test_network_push__wildcards(void)
-{
-	/* Wildcards are not rejected by git_push_add_refspec(), but aren't resolved
-	 * either
-	 */
-	const char *specs_wild1[] = { "refs/heads/*:refs/heads/*" };
-	const char *specs_wild2[] = { "refs/heads/*:refs/heads/master" };
-	const char *specs_wild3[] = { "refs/heads/master:refs/heads/*" };
-	push_status exp_stats_wild3[] = { { "refs/heads/*", "funny refname" } };
-	const char *specs_wild4[] = { "refs/heads/*:refs/heads/master/*" };
-	const char *specs_wild5[] = { "refs/heads/m*ster:refs/heads/master" };
-	const char *specs_wild6[] = { "refs/heads/master:refs/heads/m*ster" };
-	push_status exp_stats_wild6[] = { { "refs/heads/m*ster", "funny refname" } };
-	const char *specs_wild7[] = { "refs/heads/m*ster:refs/heads/m*ster" };
-
-	do_push(specs_wild1, ARRAY_SIZE(specs_wild1), NULL, 0, NULL, 0, -1);
-	do_push(specs_wild2, ARRAY_SIZE(specs_wild2), NULL, 0, NULL, 0, -1);
-	do_push(specs_wild3, ARRAY_SIZE(specs_wild3),
-		exp_stats_wild3, ARRAY_SIZE(exp_stats_wild3),
-		NULL, 0, 0);
-	do_push(specs_wild4, ARRAY_SIZE(specs_wild4), NULL, 0, NULL, 0, -1);
-	do_push(specs_wild5, ARRAY_SIZE(specs_wild5), NULL, 0, NULL, 0, -1);
-	do_push(specs_wild6, ARRAY_SIZE(specs_wild6),
-		exp_stats_wild6, ARRAY_SIZE(exp_stats_wild6),
-		NULL, 0, 0);
-	do_push(specs_wild7, ARRAY_SIZE(specs_wild7), NULL, 0, NULL, 0, -1);
-}
+}
\ No newline at end of file
diff --git a/tests-clar/network/push_util.c b/tests-clar/network/push_util.c
index e9dbfaafa..2e457844d 100644
--- a/tests-clar/network/push_util.c
+++ b/tests-clar/network/push_util.c
@@ -54,10 +54,11 @@ int delete_ref_cb(git_remote_head *head, void *payload)
 		return 0;
 
 	/* Create a refspec that deletes a branch in the remote */
-	cl_git_pass(git_buf_putc(&del_spec, ':'));
-	cl_git_pass(git_buf_puts(&del_spec, head->name));
-
-	cl_git_pass(git_vector_insert(delete_specs, git_buf_detach(&del_spec)));
+	if (strcmp(head->name, "refs/heads/master")) {
+		cl_git_pass(git_buf_putc(&del_spec, ':'));
+		cl_git_pass(git_buf_puts(&del_spec, head->name));
+		cl_git_pass(git_vector_insert(delete_specs, git_buf_detach(&del_spec)));
+	}
 
 	return 0;
 }
@@ -70,22 +71,32 @@ int record_ref_cb(git_remote_head *head, void *payload)
 
 void verify_remote_refs(git_vector *actual_refs, const expected_ref expected_refs[], size_t expected_refs_len)
 {
-	size_t i;
+	size_t i, j = 0;
 	git_buf msg = GIT_BUF_INIT;
 	git_remote_head *actual;
 	char *oid_str;
+	bool master_present = false;
+
+	/* We don't care whether "master" is present on the other end or not */
+	git_vector_foreach(actual_refs, i, actual) {
+		if (!strcmp(actual->name, "refs/heads/master")) {
+			master_present = true;
+			break;
+		}
+	}
 
-	if (expected_refs_len != actual_refs->length)
+	if (expected_refs_len + (master_present ? 1 : 0) != actual_refs->length)
 		goto failed;
 
-	for(i = 0; i < expected_refs_len; i++) {
-		git_remote_head *curr_actual = git_vector_get(actual_refs, i);
+	git_vector_foreach(actual_refs, i, actual) {
+		if (master_present && !strcmp(actual->name, "refs/heads/master"))
+			continue;
 
-		if (
-			strcmp(expected_refs[i].name, curr_actual->name)
-			|| git_oid_cmp(expected_refs[i].oid, &curr_actual->oid)
-		)
+		if (strcmp(expected_refs[j].name, actual->name) ||
+			git_oid_cmp(expected_refs[j].oid, &actual->oid))
 			goto failed;
+
+		j++;
 	}
 
 	return;
@@ -101,6 +112,9 @@ failed:
 
 	git_buf_puts(&msg, "\nACTUAL:\n");
 	git_vector_foreach(actual_refs, i, actual) {
+		if (master_present && !strcmp(actual->name, "refs/heads/master"))
+			continue;
+
 		cl_assert(oid_str = git_oid_allocfmt(&actual->oid));
 		cl_git_pass(git_buf_printf(&msg, "%s = %s\n", actual->name, oid_str));
 		git__free(oid_str);
diff --git a/tests-clar/resources/push_src/.gitted/objects/pack/dummy b/tests-clar/resources/push_src/.gitted/objects/pack/dummy
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/tests-clar/resources/push_src/.gitted/objects/pack/dummy
diff --git a/tests-clar/resources/push_src/.gitted/refs/heads/master b/tests-clar/resources/push_src/.gitted/refs/heads/b6
index 711e466ae..711e466ae 100644
--- a/tests-clar/resources/push_src/.gitted/refs/heads/master
+++ b/tests-clar/resources/push_src/.gitted/refs/heads/b6