Commit 8178c70ff43efdc0f176f32a7b6e5c8f55f3e67c

Patrick Steinhardt 2018-06-06T09:23:01

tests: submodule: do not rely on config iteration order The test submodule::lookup::duplicated_path, which tries to verify that we detect submodules with duplicated paths, currently relies on the gitmodules file of "submod2_target". While this file has two gitmodules with the same path, one of these gitmodules has an empty name and thus does not pass `git_submodule_name_is_valid`. Because of this, the test is in fact dependent on the iteration order in which we process the submodules. In fact the "valid" submodule comes first, the "invalid" submodule will cause the desired error. In fact the "invalid" submodule comes first, it will be skipped due to its name being invalid, and we will not see the desired error. While this works on the master branch just right due to the refactoring of our config code, where iteration order is now deterministic, this breaks on all older maintenance branches. Fix the issue by simply using `cl_git_rewritefile` to rewrite the gitmodules file. This greatly simplifies the test and also makes the intentions of it much clearer.

diff --git a/tests/submodule/lookup.c b/tests/submodule/lookup.c
index 5db5c2d..f73ca26 100644
--- a/tests/submodule/lookup.c
+++ b/tests/submodule/lookup.c
@@ -132,7 +132,7 @@ void test_submodule_lookup__foreach(void)
 	cl_assert_equal_i(8, data.count);
 }
 
-static int sm_dummy_cb(git_submodule *sm, const char *name, void *payload)
+static int foreach_cb(git_submodule *sm, const char *name, void *payload)
 {
 	GIT_UNUSED(sm);
 	GIT_UNUSED(name);
@@ -142,20 +142,15 @@ static int sm_dummy_cb(git_submodule *sm, const char *name, void *payload)
 
 void test_submodule_lookup__duplicated_path(void)
 {
-	/*
-	 * Manually invoke cleanup methods to remove leftovers
-	 * from `setup_fixture_submod2`
-	 */
-	cl_git_sandbox_cleanup();
-	cl_fixture_cleanup("submod2_target");
-
-	g_repo = setup_fixture_submodules();
+	cl_git_rewritefile("submod2/.gitmodules",
+			   "[submodule \"sm1\"]\n"
+			   "    path = duplicated-path\n"
+			   "    url = sm1\n"
+			   "[submodule \"sm2\"]\n"
+			   "    path = duplicated-path\n"
+			   "    url = sm2\n");
 
-	/*
-	 * This should fail, as the submodules repo has an
-	 * invalid gitmodules file with duplicated paths.
-	 */
-	cl_git_fail(git_submodule_foreach(g_repo, sm_dummy_cb, NULL));
+	cl_git_fail(git_submodule_foreach(g_repo, foreach_cb, NULL));
 }
 
 void test_submodule_lookup__lookup_even_with_unborn_head(void)
@@ -456,14 +451,6 @@ void test_submodule_lookup__lookup_in_bare_repository_fails(void)
 	cl_git_fail(git_submodule_lookup(&sm, g_repo, "nonexisting"));
 }
 
-static int foreach_cb(git_submodule *sm, const char *name, void *payload)
-{
-	GIT_UNUSED(sm);
-	GIT_UNUSED(name);
-	GIT_UNUSED(payload);
-	return 0;
-}
-
 void test_submodule_lookup__foreach_in_bare_repository_fails(void)
 {
 	cl_git_sandbox_cleanup();