Skip to content

tests: submodule: do not rely on config iteration order#4673

Merged
pks-t merged 1 commit intolibgit2:masterfrom
pks-t:pks/submodule-dupes-simplify-test
Jun 6, 2018
Merged

tests: submodule: do not rely on config iteration order#4673
pks-t merged 1 commit intolibgit2:masterfrom
pks-t:pks/submodule-dupes-simplify-test

Conversation

@pks-t
Copy link
Copy Markdown
Member

@pks-t pks-t commented Jun 6, 2018

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.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant