Re: [PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> Currently lib-submodule-update.sh provides 2 functions
> test_submodule_switch and test_submodule_forced_switch that are used by a
> variety of tests to ensure that submodules behave as expected. The current
> expected behavior is that submodules are not touched at all (see
> 42639d2317a for the exact setup).
>
> In the future we want to teach all these commands to properly recurse
> into submodules. To do that, we'll add two testing functions to
> submodule-update-lib.sh test_submodule_switch_recursing and
> test_submodule_forced_switch_recursing.

I'd remove "properly" and insert a colon after "add two ... to
submodule-update-lib.sh" before the names of two functions.

> +reset_work_tree_to_interested () {
> +	reset_work_tree_to $1 &&
> +	# indicate we are interested in the submodule:
> +	git -C submodule_update config submodule.sub1.url "bogus" &&
> +	# also have it available:
> +	if ! test -d submodule_update/.git/modules/sub1
> +	then
> +		mkdir submodule_update/.git/modules &&

Would we want "mkdir -p" here to be safe?

> +		cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1

... ahh, wouldn't matter that much, we checked that modules/sub1
does not exist, and as long as nobody creates modules/ or modules/somethingelse
we are OK.

> +	fi
> +}
> +

> @@ -695,3 +736,443 @@ test_submodule_forced_switch () {
>  		)
>  	'
>  }
> +
> +# Test that submodule contents are correctly updated when switching
> +# between commits that change a submodule.
> +# Test that the following transitions are correctly handled:
> +# (These tests are also above in the case where we expect no change
> +#  in the submodule)
> +# - Updated submodule
> +# - New submodule
> +# - Removed submodule
> +# - Directory containing tracked files replaced by submodule
> +# - Submodule replaced by tracked files in directory
> +# - Submodule replaced by tracked file with the same name
> +# - tracked file replaced by submodule

These should work without trouble only when the paths involved in
the operation in the working tree are clean, right?  Just double
checking.  If they are dirty we should expect a failure, instead of
silent loss of information.

> +# New test cases
> +# - Removing a submodule with a git directory absorbs the submodules
> +#   git directory first into the superproject.
> +
> +test_submodule_switch_recursing () {
> +	command="$1"

The dq-pair is not strictly needed on the RHS of the assignment, but
it is a good way to signal that we considered that we might receive
an argument with $IFS in it...

> +	######################### Appearing submodule #########################
> +	# Switching to a commit letting a submodule appear checks it out ...
> +	test_expect_success "$command: added submodule is checked out" '
> +		prolog &&
> +		reset_work_tree_to_interested no_submodule &&
> +		(
> +			cd submodule_update &&
> +			git branch -t add_sub1 origin/add_sub1 &&
> +			$command add_sub1 &&

... and after doing so, not quoting $command here signals that we
expect command line splitting to happen.  Am I reading it correctly?
Without an actual caller appearing in this step, it is rather hard
to judge.

> +			test_superproject_content origin/add_sub1 &&
> +			test_submodule_content sub1 origin/add_sub1
> +		)
> ...
> +	# ... but an ignored file is fine.
> +	test_expect_success "$command: added submodule removes an untracked ignored file" '
> +		test_when_finished "rm submodule_update/.git/info/exclude" &&
> +		prolog &&
> +		reset_work_tree_to_interested no_submodule &&
> +		(
> +			cd submodule_update &&
> +			git branch -t add_sub1 origin/add_sub1 &&
> +			: >sub1 &&
> +			echo sub1 > .git/info/exclude

    ">.git/info/exclude"

> +			$command add_sub1 &&
> +			test_superproject_content origin/add_sub1 &&
> +			test_submodule_content sub1 origin/add_sub1
> +		)
> +	'