{"thread":{"id":"32981","subject":"[PATCH] t7502: perform commits using alternate editor in a subshell","startedAt":"2013-02-22T23:13:00Z","lastAt":"2013-02-22T23:24:28Z","messageCount":2,"participants":["Brandon Casey","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210073","messageId":"1361574780-30067-1-git-send-email-bcasey@nvidia.com","threadId":"32981","inReplyTo":null,"subject":"[PATCH] t7502: perform commits using alternate editor in a subshell","fromName":"Brandon Casey","fromEmail":"bcasey@nvidia.com","sentAt":"2013-02-22T23:13:00Z","receivedAt":"2013-02-22T23:13:00Z","isPatch":true,"sender":{"key":"bcasey@nvidia.com","avatar":null},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThese tests call test_set_editor to set an alternate editor script, but\nthey appear to presume that the assignment is of a temporary nature and\nwill not have any effect outside of each individual test.  That is not\nthe case.  All of the test functions within a test script share a single\nenvironment, so any variables modified in one, are visible in the ones\nthat follow.\n\nSo, let's protect the test functions that follow these, which set an\nalternate editor, by performing the test_set_editor and 'git commit'\nin a subshell.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n\n\nBefore \"git-commit: populate the edit buffer with 2 blank lines before s-o-b\"\nis merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that\nthe default EDITOR remains in effect for the new test.\n\n-Brandon\n\n\n t/t7502-commit.sh | 24 ++++++++++++++++--------\n 1 file changed, 16 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\nindex b1c7648..520a5cd 100755\n--- a/t/t7502-commit.sh\n+++ b/t/t7502-commit.sh\n@@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config\n test_expect_success 'cleanup commit message (no config and no option uses default)' '\n \techo content >>file &&\n \tgit add file &&\n-\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n-\tgit commit --no-status &&\n+\t(\n+\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n+\t  git commit --no-status\n+\t) &&\n \tcommit_msg_is \"commit message\"\n '\n \n test_expect_success 'cleanup commit message (option overrides default)' '\n \techo content >>file &&\n \tgit add file &&\n-\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n-\tgit commit --cleanup=whitespace --no-status &&\n+\t(\n+\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n+\t  git commit --cleanup=whitespace --no-status\n+\t) &&\n \tcommit_msg_is \"commit message # comment\"\n '\n \n test_expect_success 'cleanup commit message (config overrides default)' '\n \techo content >>file &&\n \tgit add file &&\n-\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n-\tgit -c commit.cleanup=whitespace commit --no-status &&\n+\t(\n+\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n+\t  git -c commit.cleanup=whitespace commit --no-status\n+\t) &&\n \tcommit_msg_is \"commit message # comment\"\n '\n \n test_expect_success 'cleanup commit message (option overrides config)' '\n \techo content >>file &&\n \tgit add file &&\n-\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n-\tgit -c commit.cleanup=whitespace commit --cleanup=default &&\n+\t(\n+\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n+\t  git -c commit.cleanup=whitespace commit --cleanup=default\n+\t) &&\n \tcommit_msg_is \"commit message\"\n '\n \n-- \n1.8.1.3.566.gaa39828\n"},{"id":"210076","messageId":"7vliagaq4z.fsf@alter.siamese.dyndns.org","threadId":"32981","inReplyTo":"1361574780-30067-1-git-send-email-bcasey@nvidia.com","subject":"Re: [PATCH] t7502: perform commits using alternate editor in a subshell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-22T23:24:28Z","receivedAt":"2013-02-22T23:24:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <bcasey@nvidia.com> writes:\n\n> From: Brandon Casey <drafnel@gmail.com>\n>\n> These tests call test_set_editor to set an alternate editor script, but\n> they appear to presume that the assignment is of a temporary nature and\n> will not have any effect outside of each individual test.  That is not\n> the case.  All of the test functions within a test script share a single\n> environment, so any variables modified in one, are visible in the ones\n> that follow.\n>\n> So, let's protect the test functions that follow these, which set an\n> alternate editor, by performing the test_set_editor and 'git commit'\n> in a subshell.\n>\n> Signed-off-by: Brandon Casey <drafnel@gmail.com>\n> ---\n>\n>\n> Before \"git-commit: populate the edit buffer with 2 blank lines before s-o-b\"\n> is merged, this is needed on top of rt/commit-cleanup-config 51fb3a3d so that\n> the default EDITOR remains in effect for the new test.\n\nYeah, what I already pushed out forces EDITOR=: for your test for\nthe same effect, but this patch clearly takes us in the right (and\nbetter) direction.\n\n>\n> -Brandon\n>\n>\n>  t/t7502-commit.sh | 24 ++++++++++++++++--------\n>  1 file changed, 16 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh\n> index b1c7648..520a5cd 100755\n> --- a/t/t7502-commit.sh\n> +++ b/t/t7502-commit.sh\n> @@ -255,32 +255,40 @@ test_expect_success 'cleanup commit message (fail on invalid cleanup mode config\n>  test_expect_success 'cleanup commit message (no config and no option uses default)' '\n>  \techo content >>file &&\n>  \tgit add file &&\n> -\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> -\tgit commit --no-status &&\n> +\t(\n> +\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> +\t  git commit --no-status\n> +\t) &&\n>  \tcommit_msg_is \"commit message\"\n>  '\n>  \n>  test_expect_success 'cleanup commit message (option overrides default)' '\n>  \techo content >>file &&\n>  \tgit add file &&\n> -\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> -\tgit commit --cleanup=whitespace --no-status &&\n> +\t(\n> +\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> +\t  git commit --cleanup=whitespace --no-status\n> +\t) &&\n>  \tcommit_msg_is \"commit message # comment\"\n>  '\n>  \n>  test_expect_success 'cleanup commit message (config overrides default)' '\n>  \techo content >>file &&\n>  \tgit add file &&\n> -\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> -\tgit -c commit.cleanup=whitespace commit --no-status &&\n> +\t(\n> +\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> +\t  git -c commit.cleanup=whitespace commit --no-status\n> +\t) &&\n>  \tcommit_msg_is \"commit message # comment\"\n>  '\n>  \n>  test_expect_success 'cleanup commit message (option overrides config)' '\n>  \techo content >>file &&\n>  \tgit add file &&\n> -\ttest_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> -\tgit -c commit.cleanup=whitespace commit --cleanup=default &&\n> +\t(\n> +\t  test_set_editor \"$TEST_DIRECTORY\"/t7500/add-content-and-comment &&\n> +\t  git -c commit.cleanup=whitespace commit --cleanup=default\n> +\t) &&\n>  \tcommit_msg_is \"commit message\"\n>  '\n"}]}