threads / patch / 32981

patcht7502: perform commits using alternate editor in a subshell

Subject: [PATCH] t7502: perform commits using alternate editor in a subshell

## tl;dr

2 messages between Feb 22, 2013 and Feb 22, 2013. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Brandon Casey· Feb 22, 2013, 23:13 UTC · lore
From: Brandon Casey <drafnel@gmail.com>

These tests call test_set_editor to set an alternate editor script, but they appear to presume that the assignment is of a temporary nature and will not have any effect outside of each individual test. That is not the case. All of the test functions within a test script share a single environment, so any variables modified in one, are visible in the ones that follow.

So, let's protect the test functions that follow these, which set an alternate editor, by performing the test_set_editor and 'git commit' in a subshell.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---

Before "git-commit: populate the edit buffer with 2 blank lines before s-o-b" is merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that the default EDITOR remains in effect for the new test.

-Brandon
 t/t7502-commit.sh | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)
Show changes to t/t7502-commit.sh +16 −8
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index b1c7648..520a5cd 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config
 test_expect_success 'cleanup commit message (no config and no option uses default)' '
 	echo content >>file &&
 	git add file &&
-	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
-	git commit --no-status &&
+	(
+	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	  git commit --no-status
+	) &&
 	commit_msg_is "commit message"
 '
 
 test_expect_success 'cleanup commit message (option overrides default)' '
 	echo content >>file &&
 	git add file &&
-	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
-	git commit --cleanup=whitespace --no-status &&
+	(
+	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	  git commit --cleanup=whitespace --no-status
+	) &&
 	commit_msg_is "commit message # comment"
 '
 
 test_expect_success 'cleanup commit message (config overrides default)' '
 	echo content >>file &&
 	git add file &&
-	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
-	git -c commit.cleanup=whitespace commit --no-status &&
+	(
+	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	  git -c commit.cleanup=whitespace commit --no-status
+	) &&
 	commit_msg_is "commit message # comment"
 '
 
 test_expect_success 'cleanup commit message (option overrides config)' '
 	echo content >>file &&
 	git add file &&
-	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
-	git -c commit.cleanup=whitespace commit --cleanup=default &&
+	(
+	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
+	  git -c commit.cleanup=whitespace commit --cleanup=default
+	) &&
 	commit_msg_is "commit message"
 '
 
-- 
1.8.1.3.566.gaa39828
Junio C Hamano· Feb 22, 2013, 23:24 UTC · re: Brandon Casey · lore

Re: [PATCH] t7502: perform commits using alternate editor in a subshell

Brandon Casey <bcasey@nvidia.com> writes:
Show 20 quoted lines
> From: Brandon Casey <drafnel@gmail.com>
>
> These tests call test_set_editor to set an alternate editor script, but
> they appear to presume that the assignment is of a temporary nature and
> will not have any effect outside of each individual test.  That is not
> the case.  All of the test functions within a test script share a single
> environment, so any variables modified in one, are visible in the ones
> that follow.
>
> So, let's protect the test functions that follow these, which set an
> alternate editor, by performing the test_set_editor and 'git commit'
> in a subshell.
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>
>
> Before "git-commit: populate the edit buffer with 2 blank lines before s-o-b"
> is merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that
> the default EDITOR remains in effect for the new test.

Yeah, what I already pushed out forces EDITOR=: for your test for the same effect, but this patch clearly takes us in the right (and better) direction.

Show 59 quoted lines
>
> -Brandon
>
>
>  t/t7502-commit.sh | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
> index b1c7648..520a5cd 100755
> --- a/t/t7502-commit.sh
> +++ b/t/t7502-commit.sh
> @@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config
>  test_expect_success 'cleanup commit message (no config and no option uses default)' '
>  	echo content >>file &&
>  	git add file &&
> -	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> -	git commit --no-status &&
> +	(
> +	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> +	  git commit --no-status
> +	) &&
>  	commit_msg_is "commit message"
>  '
>  
>  test_expect_success 'cleanup commit message (option overrides default)' '
>  	echo content >>file &&
>  	git add file &&
> -	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> -	git commit --cleanup=whitespace --no-status &&
> +	(
> +	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> +	  git commit --cleanup=whitespace --no-status
> +	) &&
>  	commit_msg_is "commit message # comment"
>  '
>  
>  test_expect_success 'cleanup commit message (config overrides default)' '
>  	echo content >>file &&
>  	git add file &&
> -	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> -	git -c commit.cleanup=whitespace commit --no-status &&
> +	(
> +	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> +	  git -c commit.cleanup=whitespace commit --no-status
> +	) &&
>  	commit_msg_is "commit message # comment"
>  '
>  
>  test_expect_success 'cleanup commit message (option overrides config)' '
>  	echo content >>file &&
>  	git add file &&
> -	test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> -	git -c commit.cleanup=whitespace commit --cleanup=default &&
> +	(
> +	  test_set_editor "$TEST_DIRECTORY"/t7500/add-content-and-comment &&
> +	  git -c commit.cleanup=whitespace commit --cleanup=default
> +	) &&
>  	commit_msg_is "commit message"
>  '

← back to recent threads