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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 16, 2017, 20:39 UTC
Message-ID: <xmqqlgt5x430.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170216003811.18273-4-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 10 quoted lines

> 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.

Show 8 quoted lines

> +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
> +}
> +

Show 17 quoted lines

> @@ -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.

Show 6 quoted lines

> +# 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...

Show 9 quoted lines

> +	######################### 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.

Show 14 quoted lines

> +			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"

Show 5 quoted lines

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