Re: [PATCH 4/4] lib-submodule-update: pass OVERWRITING_FAIL

Denton Liu [off-list ref] writes:

We are using `test_must_fail $command`. However, $command is not
necessarily a git command; it could be a test helper function.

In an effort to stop using test_must_fail with non-git commands, instead
of invoking `test_must_fail $command`, run
`OVERWRITING_FAIL=test_must_fail $command` instead. Increase the
granularity of the test helper functions by specifically choosing the
individual git invocation which is designed to fail.

Sorry, but I do not know why this is a good idea.

Here, $command may or may not be a git command and more importantly,
it could be a shell function, right?  Then we need to take it into
account that 

	VAR=VAL shell_function args...

will not work, no?

Some shells do not make this a single-shot environment variable
assignment that will not persist once the single function invocation
returns.

Hi Junio,

On Wed, Apr 29, 2020 at 12:24:48PM -0700, Junio C Hamano wrote:

Denton Liu [off-list ref] writes:

quoted

We are using `test_must_fail $command`. However, $command is not
necessarily a git command; it could be a test helper function.

In an effort to stop using test_must_fail with non-git commands, instead
of invoking `test_must_fail $command`, run
`OVERWRITING_FAIL=test_must_fail $command` instead. Increase the
granularity of the test helper functions by specifically choosing the
individual git invocation which is designed to fail.

Sorry, but I do not know why this is a good idea.

This is useful because currently, when we run a test helper function, we
just mark the whole thing as `test_must_fail`. However, it's possible
that the helper function might fail earlier or later than expected due
to some side effect. If this happens, then the test case will still
be reported as passing but I think that it should be marked as failing
instead since it didn't actually display the desired behaviour.

quoted

 test_submodule_switch_common () {
+	OVERWRITING_FAIL=
 	command="$1"
 	######################### Appearing submodule #########################
 	# Switching to a commit letting a submodule appear creates empty dir ...

@@ -443,7 +446,7 @@ test_submodule_switch_common () {
 		(
 			cd submodule_update &&
 			git branch -t replace_sub1_with_directory origin/replace_sub1_with_directory &&
-			test_must_fail $command replace_sub1_with_directory &&
+			OVERWRITING_FAIL=test_must_fail $command replace_sub1_with_directory &&

Here, $command may or may not be a git command and more importantly,
it could be a shell function, right?  Then we need to take it into
account that 

	VAR=VAL shell_function args...

will not work, no?

Some shells do not make this a single-shot environment variable
assignment that will not persist once the single function invocation
returns.

I looked through POSIX specification and it says under 2.9.1 Simple
Comamnds,

	If no command name results, variable assignments shall affect
	the current execution environment. Otherwise, the variable
	assignments shall be exported for the execution environment of
	the command and shall not affect the current execution
	environment (except for special built-ins).

which makes me suspect that these shells are not POSIX-compliant. What
are some examples of shells that behave this way?

That being said, I know that we live in the real world and POSIX
standards don't justify breaking test cases. I'll reroll this to add an
`OVERWRITING_FAIL=` after the command to ensure that the variable is
cleared.

Thanks,

Denton