{"thread":{"id":"55687","subject":"[PATCH 1/2] t/README: document test_config","startedAt":"2021-05-14T06:55:26Z","lastAt":"2021-05-17T07:44:43Z","messageCount":15,"participants":["Firmin Martin","Eric Sunshine","Felipe Contreras","Junio C Hamano","Bagas Sanjaya"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"424546","messageId":"20210514065508.247044-1-firminmartin24@gmail.com","threadId":"55687","inReplyTo":null,"subject":"[PATCH 1/2] t/README: document test_config","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-14T06:55:07Z","receivedAt":"2021-05-14T06:55:26Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"test_config is used over one thousand times in the test suite, yet not\ndocumented. Give it a place in the \"Test harness library\" section.\n\nSigned-off-by: Firmin Martin <firminmartin24@gmail.com>\n---\n t/README | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex 8eb9e46b1d..f69aa05c61 100644\n--- a/t/README\n+++ b/t/README\n@@ -1046,6 +1046,21 @@ library for your script to use.\n    Abort the test script if either the value of the variable or the\n    default are not valid bool values.\n \n+ - test_config <config-option> [<value>]\n+\n+   Set the configuration option <config-option> to <value>, and unset it at the\n+   end of the current test. For a similar purpose, test_config_global for\n+   global configuration is also available. Note, however, that test_config_*\n+   should not be used under a subshell.\n+  \n+   Example:\n+\n+\ttest_config format.coverLetter auto\n+\n+   Is a concise way to write:\n+\ttest_when_finished \"git config --unset format.coverLetter\" &&\n+\tgit config format.coverLetter auto\n+\n \n Prerequisites\n -------------\n-- \n2.31.1.443.g67a420f573\n\n"},{"id":"424547","messageId":"20210514065508.247044-2-firminmartin24@gmail.com","threadId":"55687","inReplyTo":"20210514065508.247044-1-firminmartin24@gmail.com","subject":"[PATCH 2/2] t: use test_config whenever possible","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-14T06:55:08Z","receivedAt":"2021-05-14T06:55:30Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Replace patterns of the form\n1.\n        git config <config-option> <value> &&\n        ...\n        git config --unset <config-option> &&\n2.\n        test_when_finished \"git config --unset <config-option>\" &&\n        git config <config-option> <value> &&\n        ...\n\nto the concise\n\n        test_config <config-option> <value>\n\nIn t5526, two tests have been further simplified as the output file is\nwritten before \"git config --global --unset\".\n\nSigned-off-by: Firmin Martin <firminmartin24@gmail.com>\n---\n t/t3200-branch.sh                |  9 +++------\n t/t3900-i18n-commit.sh           |  3 +--\n t/t4027-diff-submodule.sh        |  3 +--\n t/t4041-diff-submodule-option.sh |  3 +--\n t/t4205-log-pretty-formats.sh    |  8 +++-----\n t/t5505-remote.sh                |  3 +--\n t/t5526-fetch-submodules.sh      | 16 ++++------------\n t/t6006-rev-list-format.sh       |  5 ++---\n t/t7401-submodule-summary.sh     |  3 +--\n t/t7610-mergetool.sh             |  2 +-\n t/t7900-maintenance.sh           |  9 +++------\n 11 files changed, 21 insertions(+), 43 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex cc4b10236e..0b0119bbe2 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -365,11 +365,9 @@ EOF\n '\n \n test_expect_success 'git branch with column.*' '\n-\tgit config column.ui column &&\n-\tgit config column.branch \"dense\" &&\n+\ttest_config column.ui column &&\n+\ttest_config column.branch \"dense\" &&\n \tCOLUMNS=80 git branch >actual &&\n-\tgit config --unset column.branch &&\n-\tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n   a/b/c   bam   foo   l   * main   n     o/p   r\n   abc     bar   j/k   m/m   mb     o/o   q     topic\n@@ -382,9 +380,8 @@ test_expect_success 'git branch --column -v should fail' '\n '\n \n test_expect_success 'git branch -v with column.ui ignored' '\n-\tgit config column.ui column &&\n+\ttest_config column.ui column &&\n \tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n-\tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n   a/b/c\n   abc\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex bfab245eb3..c16c0f7fba 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -179,7 +179,7 @@ test_commit_autosquash_flags () {\n \tH=$1\n \tflag=$2\n \ttest_expect_success \"commit --$flag with $H encoding\" '\n-\t\tgit config i18n.commitencoding $H &&\n+\t\ttest_config i18n.commitencoding $H &&\n \t\tgit checkout -b $H-$flag C0 &&\n \t\techo $H >>F &&\n \t\tgit commit -a -F \"$TEST_DIRECTORY\"/t3900/$H.txt &&\n@@ -193,7 +193,6 @@ test_commit_autosquash_flags () {\n \t\tE=$(git cat-file commit '$H-$flag' |\n \t\t\tsed -ne \"s/^encoding //p\") &&\n \t\ttest \"z$E\" = \"z$H\" &&\n-\t\tgit config --unset-all i18n.commitencoding &&\n \t\tgit rebase --autosquash -i HEAD^^^ &&\n \t\tgit log --oneline >actual &&\n \t\ttest_line_count = 3 actual\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 94ef77e1df..a6eb2416ed 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -123,7 +123,7 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'\n '\n \n test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '\n-\tgit config diff.ignoreSubmodules dirty &&\n+\ttest_config diff.ignoreSubmodules dirty &&\n \tgit diff HEAD >actual &&\n \ttest_must_be_empty actual &&\n \tgit config --add -f .gitmodules submodule.subname.ignore none &&\n@@ -152,7 +152,6 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)\n \ttest_cmp expect.body actual.body &&\n \tgit config --remove-section submodule.subname &&\n \tgit config --remove-section -f .gitmodules submodule.subname &&\n-\tgit config --unset diff.ignoreSubmodules &&\n \trm .gitmodules\n '\n \ndiff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\nindex 0c1502d4b0..782424c2d0 100755\n--- a/t/t4041-diff-submodule-option.sh\n+++ b/t/t4041-diff-submodule-option.sh\n@@ -58,13 +58,12 @@ test_expect_success 'added submodule' '\n '\n \n test_expect_success 'added submodule, set diff.submodule' '\n-\tgit config diff.submodule log &&\n+\ttest_config diff.submodule log &&\n \tgit add sm1 &&\n \tgit diff --cached >actual &&\n \tcat >expected <<-EOF &&\n \tSubmodule sm1 0000000...$head1 (new submodule)\n \tEOF\n-\tgit config --unset diff.submodule &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex cabdf7d57a..6ad232cb40 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -30,12 +30,11 @@ test_expect_success 'set up basic repos' '\n \t>bar &&\n \tgit add foo &&\n \ttest_tick &&\n-\tgit config i18n.commitEncoding $test_encoding &&\n+\ttest_config i18n.commitEncoding $test_encoding &&\n \tcommit_msg $test_encoding | git commit -F - &&\n \tgit add bar &&\n \ttest_tick &&\n-\tgit commit -m \"add bar\" &&\n-\tgit config --unset i18n.commitEncoding\n+\tgit commit -m \"add bar\"\n '\n \n test_expect_success 'alias builtin format' '\n@@ -60,10 +59,9 @@ test_expect_success 'alias user-defined format' '\n '\n \n test_expect_success 'alias user-defined tformat with %s (ISO8859-1 encoding)' '\n-\tgit config i18n.logOutputEncoding $test_encoding &&\n+\ttest_config i18n.logOutputEncoding $test_encoding &&\n \tgit log --oneline >expected-s &&\n \tgit log --pretty=\"tformat:%h %s\" >actual-s &&\n-\tgit config --unset i18n.logOutputEncoding &&\n \ttest_cmp expected-s actual-s\n '\n \ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c7b392794b..bed8e6633a 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -836,8 +836,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '\n \n test_expect_success 'rename succeeds with existing remote.<target>.prune' '\n \tgit clone one four.four &&\n-\ttest_when_finished git config --global --unset remote.upstream.prune &&\n-\tgit config --global remote.upstream.prune true &&\n+\ttest_config_global remote.upstream.prune true &&\n \tgit -C four.four remote rename origin upstream\n '\n \ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex ed11569d8d..ff18263171 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -418,7 +418,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \t\tgit fetch --recurse-submodules\n \t) &&\n \tadd_upstream_commit &&\n-\tgit config --global fetch.recurseSubmodules false &&\n+\ttest_config_global fetch.recurseSubmodules false &&\n \thead1=$(git rev-parse --short HEAD) &&\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n@@ -429,11 +429,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \t(\n \t\tcd downstream &&\n \t\tgit config fetch.recurseSubmodules on-demand &&\n-\t\tgit fetch >../actual.out 2>../actual.err\n-\t) &&\n-\tgit config --global --unset fetch.recurseSubmodules &&\n-\t(\n-\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err &&\n \t\tgit config --unset fetch.recurseSubmodules\n \t) &&\n \ttest_must_be_empty actual.out &&\n@@ -446,7 +442,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n \t\tgit fetch --recurse-submodules\n \t) &&\n \tadd_upstream_commit &&\n-\tgit config fetch.recurseSubmodules false &&\n+\ttest_config fetch.recurseSubmodules false &&\n \thead1=$(git rev-parse --short HEAD) &&\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n@@ -457,11 +453,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n \t(\n \t\tcd downstream &&\n \t\tgit config submodule.submodule.fetchRecurseSubmodules on-demand &&\n-\t\tgit fetch >../actual.out 2>../actual.err\n-\t) &&\n-\tgit config --unset fetch.recurseSubmodules &&\n-\t(\n-\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err &&\n \t\tgit config --unset submodule.submodule.fetchRecurseSubmodules\n \t) &&\n \ttest_must_be_empty actual.out &&\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 35a2f62392..9ba06ec5ae 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -37,7 +37,7 @@ truncate_count=20\n test_expect_success 'setup' '\n \t: >foo &&\n \tgit add foo &&\n-\tgit config i18n.commitEncoding $test_encoding &&\n+\ttest_config i18n.commitEncoding $test_encoding &&\n \techo \"$added_iso88591\" | git commit -F - &&\n \thead1=$(git rev-parse --verify HEAD) &&\n \thead1_short=$(git rev-parse --verify --short $head1) &&\n@@ -48,8 +48,7 @@ test_expect_success 'setup' '\n \thead2=$(git rev-parse --verify HEAD) &&\n \thead2_short=$(git rev-parse --verify --short $head2) &&\n \ttree2=$(git rev-parse --verify HEAD:) &&\n-\ttree2_short=$(git rev-parse --verify --short $tree2) &&\n-\tgit config --unset i18n.commitEncoding\n+\ttree2_short=$(git rev-parse --verify --short $tree2)\n '\n \n # usage: test_format name format_string [failure] <expected_output\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 9c3cc4cf40..4963f3290d 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -116,7 +116,7 @@ test_expect_success 'no ignore=all setting has any effect' \"\n \tgit config -f .gitmodules submodule.sm1.path sm1 &&\n \tgit config -f .gitmodules submodule.sm1.ignore all &&\n \tgit config submodule.sm1.ignore all &&\n-\tgit config diff.ignoreSubmodules all &&\n+\ttest_config diff.ignoreSubmodules all &&\n \tgit submodule summary >actual &&\n \tcat >expected <<-EOF &&\n \t* sm1 $head1...$head2 (1):\n@@ -124,7 +124,6 @@ test_expect_success 'no ignore=all setting has any effect' \"\n \n \tEOF\n \ttest_cmp expected actual &&\n-\tgit config --unset diff.ignoreSubmodules &&\n \tgit config --remove-section submodule.sm1 &&\n \tgit config -f .gitmodules --remove-section submodule.sm1\n \"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 8cc64729ad..2ff00f6c46 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -803,7 +803,7 @@ test_expect_success 'diff.orderFile configuration is honored' '\n test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_when_finished \"git reset --hard\" &&\n \tgit checkout -b test$test_count order-file-side2 &&\n-\ttest_config diff.orderFile order-file &&\n+\tgit config diff.orderFile order-file &&\n \ttest_config mergetool.myecho.cmd \"echo \\\"\\$LOCAL\\\"\" &&\n \ttest_config mergetool.myecho.trustExitCode true &&\n \techo b >order-file &&\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex b93ae014ee..7f12619feb 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -155,8 +155,7 @@ test_expect_success 'prefetch multiple remotes' '\n \tgit log --oneline --decorate --all >log &&\n \t! grep \"prefetch\" log &&\n \n-\ttest_when_finished git config --unset remote.remote1.skipFetchAll &&\n-\tgit config remote.remote1.skipFetchAll true &&\n+\ttest_config remote.remote1.skipFetchAll true &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/skip-remote1.txt\" git maintenance run --task=prefetch 2>/dev/null &&\n \ttest_subcommand ! git fetch remote1 $fetchargs <skip-remote1.txt &&\n \ttest_subcommand git fetch remote2 $fetchargs <skip-remote1.txt\n@@ -407,8 +406,7 @@ test_expect_success 'maintenance.strategy inheritance' '\n \t\tgit config --unset maintenance.$task.schedule || return 1\n \tdone &&\n \n-\ttest_when_finished git config --unset maintenance.strategy &&\n-\tgit config maintenance.strategy incremental &&\n+\ttest_config maintenance.strategy incremental &&\n \n \tGIT_TRACE2_EVENT=\"$(pwd)/incremental-hourly.txt\" \\\n \t\tgit maintenance run --schedule=hourly --quiet &&\n@@ -465,8 +463,7 @@ test_expect_success 'maintenance.strategy inheritance' '\n '\n \n test_expect_success 'register and unregister' '\n-\ttest_when_finished git config --global --unset-all maintenance.repo &&\n-\tgit config --global --add maintenance.repo /existing1 &&\n+\ttest_config_global maintenance.repo /existing1 &&\n \tgit config --global --add maintenance.repo /existing2 &&\n \tgit config --global --get-all maintenance.repo >before &&\n \n-- \n2.31.1.443.g67a420f573\n\n"},{"id":"424548","messageId":"CAPig+cRrVN0cqoRFA90XYSsDcJ-PZxLMNfFCye4pUXz47AhpYw@mail.gmail.com","threadId":"55687","inReplyTo":"20210514065508.247044-1-firminmartin24@gmail.com","subject":"Re: [PATCH 1/2] t/README: document test_config","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-05-14T07:02:42Z","receivedAt":"2021-05-14T07:02:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, May 14, 2021 at 2:55 AM Firmin Martin <firminmartin24@gmail.com> wrote:\n> test_config is used over one thousand times in the test suite, yet not\n> documented. Give it a place in the \"Test harness library\" section.\n>\n> Signed-off-by: Firmin Martin <firminmartin24@gmail.com>\n> ---\n> diff --git a/t/README b/t/README\n> @@ -1046,6 +1046,21 @@ library for your script to use.\n> + - test_config <config-option> [<value>]\n> +\n> +   Set the configuration option <config-option> to <value>, and unset it at the\n> +   end of the current test. For a similar purpose, test_config_global for\n> +   global configuration is also available. Note, however, that test_config_*\n> +   should not be used under a subshell.\n\n\"should\" is perhaps too weak of a word. test_config() will not\nfunction correctly at all (just as test_when_finished() will not\nfunction correctly) within a subshell. So, perhaps say \"must not be\nused in a subshell.\"\n"},{"id":"424658","messageId":"87mtswf462.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55687","inReplyTo":"CAPig+cRrVN0cqoRFA90XYSsDcJ-PZxLMNfFCye4pUXz47AhpYw@mail.gmail.com","subject":"Re: [PATCH 1/2] t/README: document test_config","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-15T14:43:49Z","receivedAt":"2021-05-15T14:43:57Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Eric,\n\nEric Sunshine <sunshine@sunshineco.com> writes:\n\n> On Fri, May 14, 2021 at 2:55 AM Firmin Martin <firminmartin24@gmail.com> wrote:\n>> test_config is used over one thousand times in the test suite, yet not\n>> documented. Give it a place in the \"Test harness library\" section.\n>>\n>> Signed-off-by: Firmin Martin <firminmartin24@gmail.com>\n>> ---\n>> diff --git a/t/README b/t/README\n>> @@ -1046,6 +1046,21 @@ library for your script to use.\n>> + - test_config <config-option> [<value>]\n>> +\n>> +   Set the configuration option <config-option> to <value>, and unset it at the\n>> +   end of the current test. For a similar purpose, test_config_global for\n>> +   global configuration is also available. Note, however, that test_config_*\n>> +   should not be used under a subshell.\n>\n> \"should\" is perhaps too weak of a word. test_config() will not\n> function correctly at all (just as test_when_finished() will not\n> function correctly) within a subshell. So, perhaps say \"must not be\n> used in a subshell.\"\nIndeed, will correct this.\n\nThanks,\n\nFirmin\n\n"},{"id":"424660","messageId":"20210515152721.885728-1-firminmartin24@gmail.com","threadId":"55687","inReplyTo":"20210514065508.247044-1-firminmartin24@gmail.com","subject":"[PATCH v2 0/2] document test_config & use it whenever possible","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-15T15:27:19Z","receivedAt":"2021-05-15T15:28:55Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"* Fix wording.\n\n---\nFirmin Martin (2):\n  t/README: document test_config\n  t: use test_config whenever possible\n\n t/README                         | 15 +++++++++++++++\n t/t3200-branch.sh                |  9 +++------\n t/t3900-i18n-commit.sh           |  3 +--\n t/t4027-diff-submodule.sh        |  3 +--\n t/t4041-diff-submodule-option.sh |  3 +--\n t/t4205-log-pretty-formats.sh    |  8 +++-----\n t/t5505-remote.sh                |  3 +--\n t/t5526-fetch-submodules.sh      | 16 ++++------------\n t/t6006-rev-list-format.sh       |  5 ++---\n t/t7401-submodule-summary.sh     |  3 +--\n t/t7610-mergetool.sh             |  2 +-\n t/t7900-maintenance.sh           |  9 +++------\n 12 files changed, 36 insertions(+), 43 deletions(-)\n\nRange-diff against v1:\n1:  ee35266247 ! 1:  07ca310549 t/README: document test_config\n    @@ Commit message\n         test_config is used over one thousand times in the test suite, yet not\n         documented. Give it a place in the \"Test harness library\" section.\n     \n    +    Thanks-to: Eric Sunshine <sunshine@sunshineco.com>\n         Signed-off-by: Firmin Martin <firminmartin24@gmail.com>\n     \n      ## t/README ##\n    @@ t/README: library for your script to use.\n     +   Set the configuration option <config-option> to <value>, and unset it at the\n     +   end of the current test. For a similar purpose, test_config_global for\n     +   global configuration is also available. Note, however, that test_config_*\n    -+   should not be used under a subshell.\n    ++   must not be used in a subshell.\n     +  \n     +   Example:\n     +\n2:  8a878755ef = 2:  279b5b3701 t: use test_config whenever possible\n-- \n2.31.1.443.g67a420f573\n\n"},{"id":"424661","messageId":"20210515152721.885728-2-firminmartin24@gmail.com","threadId":"55687","inReplyTo":"20210515152721.885728-1-firminmartin24@gmail.com","subject":"[PATCH v2 1/2] t/README: document test_config","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-15T15:27:20Z","receivedAt":"2021-05-15T15:28:57Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"test_config is used over one thousand times in the test suite, yet not\ndocumented. Give it a place in the \"Test harness library\" section.\n\nThanks-to: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Firmin Martin <firminmartin24@gmail.com>\n---\n t/README | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/t/README b/t/README\nindex 8eb9e46b1d..aaf1c9cadc 100644\n--- a/t/README\n+++ b/t/README\n@@ -1046,6 +1046,21 @@ library for your script to use.\n    Abort the test script if either the value of the variable or the\n    default are not valid bool values.\n \n+ - test_config <config-option> [<value>]\n+\n+   Set the configuration option <config-option> to <value>, and unset it at the\n+   end of the current test. For a similar purpose, test_config_global for\n+   global configuration is also available. Note, however, that test_config_*\n+   must not be used in a subshell.\n+  \n+   Example:\n+\n+\ttest_config format.coverLetter auto\n+\n+   Is a concise way to write:\n+\ttest_when_finished \"git config --unset format.coverLetter\" &&\n+\tgit config format.coverLetter auto\n+\n \n Prerequisites\n -------------\n-- \n2.31.1.443.g67a420f573\n\n"},{"id":"424662","messageId":"20210515152721.885728-3-firminmartin24@gmail.com","threadId":"55687","inReplyTo":"20210515152721.885728-1-firminmartin24@gmail.com","subject":"[PATCH v2 2/2] t: use test_config whenever possible","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-15T15:27:21Z","receivedAt":"2021-05-15T15:28:58Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Replace patterns of the form\n1.\n        git config <config-option> <value> &&\n        ...\n        git config --unset <config-option> &&\n2.\n        test_when_finished \"git config --unset <config-option>\" &&\n        git config <config-option> <value> &&\n        ...\n\nto the concise\n\n        test_config <config-option> <value>\n\nIn t5526, two tests have been further simplified as the output file is\nwritten before \"git config --global --unset\".\n\nSigned-off-by: Firmin Martin <firminmartin24@gmail.com>\n---\n t/t3200-branch.sh                |  9 +++------\n t/t3900-i18n-commit.sh           |  3 +--\n t/t4027-diff-submodule.sh        |  3 +--\n t/t4041-diff-submodule-option.sh |  3 +--\n t/t4205-log-pretty-formats.sh    |  8 +++-----\n t/t5505-remote.sh                |  3 +--\n t/t5526-fetch-submodules.sh      | 16 ++++------------\n t/t6006-rev-list-format.sh       |  5 ++---\n t/t7401-submodule-summary.sh     |  3 +--\n t/t7610-mergetool.sh             |  2 +-\n t/t7900-maintenance.sh           |  9 +++------\n 11 files changed, 21 insertions(+), 43 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex cc4b10236e..0b0119bbe2 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -365,11 +365,9 @@ EOF\n '\n \n test_expect_success 'git branch with column.*' '\n-\tgit config column.ui column &&\n-\tgit config column.branch \"dense\" &&\n+\ttest_config column.ui column &&\n+\ttest_config column.branch \"dense\" &&\n \tCOLUMNS=80 git branch >actual &&\n-\tgit config --unset column.branch &&\n-\tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n   a/b/c   bam   foo   l   * main   n     o/p   r\n   abc     bar   j/k   m/m   mb     o/o   q     topic\n@@ -382,9 +380,8 @@ test_expect_success 'git branch --column -v should fail' '\n '\n \n test_expect_success 'git branch -v with column.ui ignored' '\n-\tgit config column.ui column &&\n+\ttest_config column.ui column &&\n \tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n-\tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n   a/b/c\n   abc\ndiff --git a/t/t3900-i18n-commit.sh b/t/t3900-i18n-commit.sh\nindex bfab245eb3..c16c0f7fba 100755\n--- a/t/t3900-i18n-commit.sh\n+++ b/t/t3900-i18n-commit.sh\n@@ -179,7 +179,7 @@ test_commit_autosquash_flags () {\n \tH=$1\n \tflag=$2\n \ttest_expect_success \"commit --$flag with $H encoding\" '\n-\t\tgit config i18n.commitencoding $H &&\n+\t\ttest_config i18n.commitencoding $H &&\n \t\tgit checkout -b $H-$flag C0 &&\n \t\techo $H >>F &&\n \t\tgit commit -a -F \"$TEST_DIRECTORY\"/t3900/$H.txt &&\n@@ -193,7 +193,6 @@ test_commit_autosquash_flags () {\n \t\tE=$(git cat-file commit '$H-$flag' |\n \t\t\tsed -ne \"s/^encoding //p\") &&\n \t\ttest \"z$E\" = \"z$H\" &&\n-\t\tgit config --unset-all i18n.commitencoding &&\n \t\tgit rebase --autosquash -i HEAD^^^ &&\n \t\tgit log --oneline >actual &&\n \t\ttest_line_count = 3 actual\ndiff --git a/t/t4027-diff-submodule.sh b/t/t4027-diff-submodule.sh\nindex 94ef77e1df..a6eb2416ed 100755\n--- a/t/t4027-diff-submodule.sh\n+++ b/t/t4027-diff-submodule.sh\n@@ -123,7 +123,7 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)'\n '\n \n test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match) [.gitmodules]' '\n-\tgit config diff.ignoreSubmodules dirty &&\n+\ttest_config diff.ignoreSubmodules dirty &&\n \tgit diff HEAD >actual &&\n \ttest_must_be_empty actual &&\n \tgit config --add -f .gitmodules submodule.subname.ignore none &&\n@@ -152,7 +152,6 @@ test_expect_success 'git diff HEAD with dirty submodule (work tree, refs match)\n \ttest_cmp expect.body actual.body &&\n \tgit config --remove-section submodule.subname &&\n \tgit config --remove-section -f .gitmodules submodule.subname &&\n-\tgit config --unset diff.ignoreSubmodules &&\n \trm .gitmodules\n '\n \ndiff --git a/t/t4041-diff-submodule-option.sh b/t/t4041-diff-submodule-option.sh\nindex 0c1502d4b0..782424c2d0 100755\n--- a/t/t4041-diff-submodule-option.sh\n+++ b/t/t4041-diff-submodule-option.sh\n@@ -58,13 +58,12 @@ test_expect_success 'added submodule' '\n '\n \n test_expect_success 'added submodule, set diff.submodule' '\n-\tgit config diff.submodule log &&\n+\ttest_config diff.submodule log &&\n \tgit add sm1 &&\n \tgit diff --cached >actual &&\n \tcat >expected <<-EOF &&\n \tSubmodule sm1 0000000...$head1 (new submodule)\n \tEOF\n-\tgit config --unset diff.submodule &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 8272d94ce6..fbe8ae5378 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -30,12 +30,11 @@ test_expect_success 'set up basic repos' '\n \t>bar &&\n \tgit add foo &&\n \ttest_tick &&\n-\tgit config i18n.commitEncoding $test_encoding &&\n+\ttest_config i18n.commitEncoding $test_encoding &&\n \tcommit_msg $test_encoding | git commit -F - &&\n \tgit add bar &&\n \ttest_tick &&\n-\tgit commit -m \"add bar\" &&\n-\tgit config --unset i18n.commitEncoding\n+\tgit commit -m \"add bar\"\n '\n \n test_expect_success 'alias builtin format' '\n@@ -60,10 +59,9 @@ test_expect_success 'alias user-defined format' '\n '\n \n test_expect_success 'alias user-defined tformat with %s (ISO8859-1 encoding)' '\n-\tgit config i18n.logOutputEncoding $test_encoding &&\n+\ttest_config i18n.logOutputEncoding $test_encoding &&\n \tgit log --oneline >expected-s &&\n \tgit log --pretty=\"tformat:%h %s\" >actual-s &&\n-\tgit config --unset i18n.logOutputEncoding &&\n \ttest_cmp expected-s actual-s\n '\n \ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex c7b392794b..bed8e6633a 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -836,8 +836,7 @@ test_expect_success 'rename a remote with name prefix of other remote' '\n \n test_expect_success 'rename succeeds with existing remote.<target>.prune' '\n \tgit clone one four.four &&\n-\ttest_when_finished git config --global --unset remote.upstream.prune &&\n-\tgit config --global remote.upstream.prune true &&\n+\ttest_config_global remote.upstream.prune true &&\n \tgit -C four.four remote rename origin upstream\n '\n \ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex ed11569d8d..ff18263171 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -418,7 +418,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \t\tgit fetch --recurse-submodules\n \t) &&\n \tadd_upstream_commit &&\n-\tgit config --global fetch.recurseSubmodules false &&\n+\ttest_config_global fetch.recurseSubmodules false &&\n \thead1=$(git rev-parse --short HEAD) &&\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n@@ -429,11 +429,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n \t(\n \t\tcd downstream &&\n \t\tgit config fetch.recurseSubmodules on-demand &&\n-\t\tgit fetch >../actual.out 2>../actual.err\n-\t) &&\n-\tgit config --global --unset fetch.recurseSubmodules &&\n-\t(\n-\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err &&\n \t\tgit config --unset fetch.recurseSubmodules\n \t) &&\n \ttest_must_be_empty actual.out &&\n@@ -446,7 +442,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n \t\tgit fetch --recurse-submodules\n \t) &&\n \tadd_upstream_commit &&\n-\tgit config fetch.recurseSubmodules false &&\n+\ttest_config fetch.recurseSubmodules false &&\n \thead1=$(git rev-parse --short HEAD) &&\n \tgit add submodule &&\n \tgit commit -m \"new submodule\" &&\n@@ -457,11 +453,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n \t(\n \t\tcd downstream &&\n \t\tgit config submodule.submodule.fetchRecurseSubmodules on-demand &&\n-\t\tgit fetch >../actual.out 2>../actual.err\n-\t) &&\n-\tgit config --unset fetch.recurseSubmodules &&\n-\t(\n-\t\tcd downstream &&\n+\t\tgit fetch >../actual.out 2>../actual.err &&\n \t\tgit config --unset submodule.submodule.fetchRecurseSubmodules\n \t) &&\n \ttest_must_be_empty actual.out &&\ndiff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh\nindex 35a2f62392..9ba06ec5ae 100755\n--- a/t/t6006-rev-list-format.sh\n+++ b/t/t6006-rev-list-format.sh\n@@ -37,7 +37,7 @@ truncate_count=20\n test_expect_success 'setup' '\n \t: >foo &&\n \tgit add foo &&\n-\tgit config i18n.commitEncoding $test_encoding &&\n+\ttest_config i18n.commitEncoding $test_encoding &&\n \techo \"$added_iso88591\" | git commit -F - &&\n \thead1=$(git rev-parse --verify HEAD) &&\n \thead1_short=$(git rev-parse --verify --short $head1) &&\n@@ -48,8 +48,7 @@ test_expect_success 'setup' '\n \thead2=$(git rev-parse --verify HEAD) &&\n \thead2_short=$(git rev-parse --verify --short $head2) &&\n \ttree2=$(git rev-parse --verify HEAD:) &&\n-\ttree2_short=$(git rev-parse --verify --short $tree2) &&\n-\tgit config --unset i18n.commitEncoding\n+\ttree2_short=$(git rev-parse --verify --short $tree2)\n '\n \n # usage: test_format name format_string [failure] <expected_output\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 9c3cc4cf40..4963f3290d 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -116,7 +116,7 @@ test_expect_success 'no ignore=all setting has any effect' \"\n \tgit config -f .gitmodules submodule.sm1.path sm1 &&\n \tgit config -f .gitmodules submodule.sm1.ignore all &&\n \tgit config submodule.sm1.ignore all &&\n-\tgit config diff.ignoreSubmodules all &&\n+\ttest_config diff.ignoreSubmodules all &&\n \tgit submodule summary >actual &&\n \tcat >expected <<-EOF &&\n \t* sm1 $head1...$head2 (1):\n@@ -124,7 +124,6 @@ test_expect_success 'no ignore=all setting has any effect' \"\n \n \tEOF\n \ttest_cmp expected actual &&\n-\tgit config --unset diff.ignoreSubmodules &&\n \tgit config --remove-section submodule.sm1 &&\n \tgit config -f .gitmodules --remove-section submodule.sm1\n \"\ndiff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh\nindex 8cc64729ad..2ff00f6c46 100755\n--- a/t/t7610-mergetool.sh\n+++ b/t/t7610-mergetool.sh\n@@ -803,7 +803,7 @@ test_expect_success 'diff.orderFile configuration is honored' '\n test_expect_success 'mergetool -Oorder-file is honored' '\n \ttest_when_finished \"git reset --hard\" &&\n \tgit checkout -b test$test_count order-file-side2 &&\n-\ttest_config diff.orderFile order-file &&\n+\tgit config diff.orderFile order-file &&\n \ttest_config mergetool.myecho.cmd \"echo \\\"\\$LOCAL\\\"\" &&\n \ttest_config mergetool.myecho.trustExitCode true &&\n \techo b >order-file &&\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex b93ae014ee..7f12619feb 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -155,8 +155,7 @@ test_expect_success 'prefetch multiple remotes' '\n \tgit log --oneline --decorate --all >log &&\n \t! grep \"prefetch\" log &&\n \n-\ttest_when_finished git config --unset remote.remote1.skipFetchAll &&\n-\tgit config remote.remote1.skipFetchAll true &&\n+\ttest_config remote.remote1.skipFetchAll true &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/skip-remote1.txt\" git maintenance run --task=prefetch 2>/dev/null &&\n \ttest_subcommand ! git fetch remote1 $fetchargs <skip-remote1.txt &&\n \ttest_subcommand git fetch remote2 $fetchargs <skip-remote1.txt\n@@ -407,8 +406,7 @@ test_expect_success 'maintenance.strategy inheritance' '\n \t\tgit config --unset maintenance.$task.schedule || return 1\n \tdone &&\n \n-\ttest_when_finished git config --unset maintenance.strategy &&\n-\tgit config maintenance.strategy incremental &&\n+\ttest_config maintenance.strategy incremental &&\n \n \tGIT_TRACE2_EVENT=\"$(pwd)/incremental-hourly.txt\" \\\n \t\tgit maintenance run --schedule=hourly --quiet &&\n@@ -465,8 +463,7 @@ test_expect_success 'maintenance.strategy inheritance' '\n '\n \n test_expect_success 'register and unregister' '\n-\ttest_when_finished git config --global --unset-all maintenance.repo &&\n-\tgit config --global --add maintenance.repo /existing1 &&\n+\ttest_config_global maintenance.repo /existing1 &&\n \tgit config --global --add maintenance.repo /existing2 &&\n \tgit config --global --get-all maintenance.repo >before &&\n \n-- \n2.31.1.443.g67a420f573\n\n"},{"id":"424665","messageId":"60a02bf2f16a2_eacf52085@natae.notmuch","threadId":"55687","inReplyTo":"20210514065508.247044-2-firminmartin24@gmail.com","subject":"RE: [PATCH 2/2] t: use test_config whenever possible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T20:15:46Z","receivedAt":"2021-05-15T20:15:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Firmin Martin wrote:\n> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\n> index ed11569d8d..ff18263171 100755\n> --- a/t/t5526-fetch-submodules.sh\n> +++ b/t/t5526-fetch-submodules.sh\n> @@ -418,7 +418,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n>  \t\tgit fetch --recurse-submodules\n>  \t) &&\n>  \tadd_upstream_commit &&\n> -\tgit config --global fetch.recurseSubmodules false &&\n> +\ttest_config_global fetch.recurseSubmodules false &&\n>  \thead1=$(git rev-parse --short HEAD) &&\n>  \tgit add submodule &&\n>  \tgit commit -m \"new submodule\" &&\n> @@ -429,11 +429,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n>  \t(\n>  \t\tcd downstream &&\n>  \t\tgit config fetch.recurseSubmodules on-demand &&\n\nUhm:\n\n  test_config fetch.recurseSubmodules on-demand &&\n\n> -\t\tgit fetch >../actual.out 2>../actual.err\n> -\t) &&\n> -\tgit config --global --unset fetch.recurseSubmodules &&\n> -\t(\n> -\t\tcd downstream &&\n> +\t\tgit fetch >../actual.out 2>../actual.err &&\n>  \t\tgit config --unset fetch.recurseSubmodules\n\n\nThen the above line can be removed too.\n\n>  \t) &&\n>  \ttest_must_be_empty actual.out &&\n> @@ -446,7 +442,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n>  \t\tgit fetch --recurse-submodules\n>  \t) &&\n>  \tadd_upstream_commit &&\n> -\tgit config fetch.recurseSubmodules false &&\n> +\ttest_config fetch.recurseSubmodules false &&\n>  \thead1=$(git rev-parse --short HEAD) &&\n>  \tgit add submodule &&\n>  \tgit commit -m \"new submodule\" &&\n> @@ -457,11 +453,7 @@ test_expect_success \"'submodule.<sub>.fetchRecurseSubmodules=on-demand' override\n>  \t(\n>  \t\tcd downstream &&\n>  \t\tgit config submodule.submodule.fetchRecurseSubmodules on-demand &&\n\nDitto.\n\nVery nice cleanup.\n\n-- \nFelipe Contreras\n"},{"id":"424666","messageId":"60a02d62838cf_eacf5208d2@natae.notmuch","threadId":"55687","inReplyTo":"60a02bf2f16a2_eacf52085@natae.notmuch","subject":"RE: [PATCH 2/2] t: use test_config whenever possible","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T20:21:54Z","receivedAt":"2021-05-15T20:22:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Felipe Contreras wrote:\n> Firmin Martin wrote:\n> > diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\n> > index ed11569d8d..ff18263171 100755\n> > --- a/t/t5526-fetch-submodules.sh\n> > +++ b/t/t5526-fetch-submodules.sh\n> > @@ -418,7 +418,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n> >  \t\tgit fetch --recurse-submodules\n> >  \t) &&\n> >  \tadd_upstream_commit &&\n> > -\tgit config --global fetch.recurseSubmodules false &&\n> > +\ttest_config_global fetch.recurseSubmodules false &&\n> >  \thead1=$(git rev-parse --short HEAD) &&\n> >  \tgit add submodule &&\n> >  \tgit commit -m \"new submodule\" &&\n> > @@ -429,11 +429,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\n> >  \t(\n> >  \t\tcd downstream &&\n> >  \t\tgit config fetch.recurseSubmodules on-demand &&\n> \n> Uhm:\n> \n>   test_config fetch.recurseSubmodules on-demand &&\n\nAhh, I just read the comment about not using it in a subshell.\n\nThen outside:\n\n  test_config -C downstream fetch.recurseSubmodules on-demand &&\n\n-- \nFelipe Contreras\n"},{"id":"424667","messageId":"87im3jfyiq.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55687","inReplyTo":"60a02d62838cf_eacf5208d2@natae.notmuch","subject":"RE: [PATCH 2/2] t: use test_config whenever possible","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-15T22:00:29Z","receivedAt":"2021-05-15T22:00:41Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":">\n> Ahh, I just read the comment about not using it in a subshell.\n>\n> Then outside:\n>\n>   test_config -C downstream fetch.recurseSubmodules on-demand &&\nTrue. This makes me realize that I should also briefly document the -C option.\n\nThanks,\n\nFirmin\n"},{"id":"424673","messageId":"xmqqbl9b6zla.fsf@gitster.g","threadId":"55687","inReplyTo":"20210515152721.885728-3-firminmartin24@gmail.com","subject":"Re: [PATCH v2 2/2] t: use test_config whenever possible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-16T05:02:09Z","receivedAt":"2021-05-16T05:02:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin <firminmartin24@gmail.com> writes:\n\n\"whenever possible\" in the subject is not wrong per-se but because\nwe do not want to accept a patch that uses it where it is impossible\nto be used anyway, it does not mean all that much.\n\nUse of test_config to replace #1 below is a correctness improvement,\nand use of test_config to replace #2 below is a readability thing.\n\n    t: use more test_config for correctness and readability\n\nperhaps?\n\n> Replace patterns of the form\n> 1.\n>         git config <config-option> <value> &&\n>         ...\n>         git config --unset <config-option> &&\n\nAdding \"because this form fails to unset the config if an earlier\nstep fails.\" would be helpful to the readers to clarify that this is\na correctness \"fix\".\n\n> 2.\n>         test_when_finished \"git config --unset <config-option>\" &&\n>         git config <config-option> <value> &&\n>         ...\n>\n> to the concise\n>\n>         test_config <config-option> <value>\n\nNice.\n\n> In t5526, two tests have been further simplified as the output file is\n> written before \"git config --global --unset\".\n\n> diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\n> index ed11569d8d..ff18263171 100755\n> --- a/t/t5526-fetch-submodules.sh\n> +++ b/t/t5526-fetch-submodules.sh\n> ...\n@@ -429,11 +429,7 @@ test_expect_success \"'fetch.recurseSu\n>  \t(\n>  \t\tcd downstream &&\n>  \t\tgit config fetch.recurseSubmodules on-demand &&\n> -\t\tgit fetch >../actual.out 2>../actual.err\n> -\t) &&\n> -\tgit config --global --unset fetch.recurseSubmodules &&\n> -\t(\n> -\t\tcd downstream &&\n> +\t\tgit fetch >../actual.out 2>../actual.err &&\n>  \t\tgit config --unset fetch.recurseSubmodules\n>  \t) &&\n>  \ttest_must_be_empty actual.out &&\n\nThe original seems to be making sure that the config set in\n\"downstream\" repository is used successfully before it is unset from\nthe global configuration and then the same fetch in the downstream\nwill work even when the configuration is removed from the global\nconfiguration.  The squashing of the two tests into one would change\nwhat is being tested, wouldn't it?\n\nSurely, the actual.out and actual.err in the first invocation *was*\noverwritten without being looked at, but the exit status of that\nfetch (i.e. fetch before global config gets unset) was still checked\nin the original.  So I'd call this a \"simplification\"\n\n>  \t(\n>  \t\tcd downstream &&\n>  \t\tgit config fetch.recurseSubmodules on-demand &&\n> -\t\tgit fetch >../actual.out 2>../actual.err\n> +\t\tgit fetch\n> \t) &&\n> \tgit config --global --unset fetch.recurseSubmodules &&\n\nI.e. these redirected output files were not used and the next fetch\nwill overwrite them.\n\nThe patch I am responding to claims that \n\n - \"make sure it succeeds with global set, and then without global\n   set, make sure it still works\" is not worth testing.\n\n - it is OK to replace it with a single \"it succeeds without\n   unsetting the global config\".\n\nIt is not clear if either is true.  Same for the other hunk that\nsquashes two tests into one for this path.\n"},{"id":"424674","messageId":"5faaa5a6-8ebf-b2ef-805f-551a36ab7e6c@gmail.com","threadId":"55687","inReplyTo":"20210515152721.885728-2-firminmartin24@gmail.com","subject":"Re: [PATCH v2 1/2] t/README: document test_config","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-16T05:03:25Z","receivedAt":"2021-05-16T05:03:31Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 15/05/21 22.27, Firmin Martin wrote:\n>   \n> + - test_config <config-option> [<value>]\n> +\n> +   Set the configuration option <config-option> to <value>, and unset it at the\n> +   end of the current test. For a similar purpose, test_config_global for\n> +   global configuration is also available. Note, however, that test_config_*\n> +   must not be used in a subshell.\n> +\n\n From the syntax above, when I omit <value>, default value for <config-option>\nwill be set, right? You forgot to mention it.\n\n> +   Example:\n> +\n> +\ttest_config format.coverLetter auto\n> +\n> +   Is a concise way to write:\n> +\ttest_when_finished \"git config --unset format.coverLetter\" &&\n> +\tgit config format.coverLetter auto\n> +\n>   \n\nThe description said \"set the config, then unset it\". But the expanded version\nsaid \"unsetting the config is deferred to the end of test and set the config\".\n\nThe documentation for test_when_finished said:\n\n>    Prepend <script> to a list of commands to run to clean up\n>    at the end of the current test.  If some clean-up command\n>    fails, the test will not pass.\n\nIs my interpretation above correct?\n\nOh yeah, maybe it's better to say:\n\"For example: <blah> have the same effect as <blah>.\"\n\nThanks.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"424702","messageId":"87fsylgadx.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55687","inReplyTo":"xmqqbl9b6zla.fsf@gitster.g","subject":"Re: [PATCH v2 2/2] t: use test_config whenever possible","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-17T06:08:42Z","receivedAt":"2021-05-17T06:09:14Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Junio,\n\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Use of test_config to replace #1 below is a correctness improvement,\n> and use of test_config to replace #2 below is a readability thing.\n>\n>     t: use more test_config for correctness and readability\n>\n> perhaps?\nOK.\n\n> Adding \"because this form fails to unset the config if an earlier\n> step fails.\" would be helpful to the readers to clarify that this is\n> a correctness \"fix\".\nWill do.\n\nTo be honest, I'm completely lost in the remaining of your reply (in\nfact, I suspect that you misread the diff, cf. the end of the email). \nI copy here the full test with line number to make sure that we are in\nthe same page.\n\n>  415 test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\" '\n>  416     (\n>  417         cd downstream &&\n>  418         git fetch --recurse-submodules\n>  419     ) &&\n>  420     add_upstream_commit &&\n>  421     git config --global fetch.recurseSubmodules false &&\n>  422     head1=$(git rev-parse --short HEAD) &&\n>  423     git add submodule &&\n>  424     git commit -m \"new submodule\" &&\n>  425     head2=$(git rev-parse --short HEAD) &&\n>  426     echo \"From $pwd/.\" > expect.err.2 &&\n>  427     echo \"   $head1..$head2  super      -> origin/super\" >>expect.err.2 &&\n>  428     head -3 expect.err >> expect.err.2 &&\n>  429     (\n>  430         cd downstream &&\n>  431         git config fetch.recurseSubmodules on-demand &&\n>  432         git fetch >../actual.out 2>../actual.err\n>  433     ) &&\n>  434     git config --global --unset fetch.recurseSubmodules &&\n>  435     (\n>  436         cd downstream &&\n>  437         git config --unset fetch.recurseSubmodules\n>  438     ) &&\n>  439     test_must_be_empty actual.out &&\n>  440     test_cmp expect.err.2 actual.err\n\n\n> The original seems to be making sure that the config set in\n> \"downstream\" repository is used successfully before it is unset from\n> the global configuration\nOK. \n> and then the same fetch in the downstream will work even when the\nWhat do you mean by \"the same fetch\"? the one of line 432 or any\ninstance of \"git -C downstream fetch\" in later tests? \n> configuration is removed from the global configuration.\n\n> The squashing of the two tests into one\nSame question as above, I don't understand \"the second test\" you are\nreferring to (specifically, which lines?).\n\n> would change what is being tested, wouldn't it?\nI agree that the semantic isn't the same (as test_config_global will\nunset global config after unsetting the \"downstream\" config), but since\nthere is *not* another git-fetch after the global unset and before the\n\"downstream\" unset I thought that it is fine.\n\n> Surely, the actual.out and actual.err in the first invocation *was*\n> overwritten without being looked at, but the exit status of that\nAren't they considered in the end of the test ? (lines 439-440).\n> fetch (i.e. fetch before global config gets unset) was still checked\n> in the original.  So I'd call this a \"simplification\"\n>\n>>  \t(\n>>  \t\tcd downstream &&\n>>  \t\tgit config fetch.recurseSubmodules on-demand &&\n>> -\t\tgit fetch >../actual.out 2>../actual.err\n>> +\t\tgit fetch\n>> \t) &&\n>> \tgit config --global --unset fetch.recurseSubmodules &&\n\n> I.e. these redirected output files were not used and the next fetch\n> will overwrite them.\nSame question as above.\n\n> The patch I am responding to claims that \n>\n>  - \"make sure it succeeds with global set, and then without global\n>    set, make sure it still works\" is not worth testing.\nIn this test, there is no \"make sure it still works without global set\"\n(i.e. I didn't see any git-fetch invocation after the global\nconfiguration unset). Are you referring later tests?\n>\n>  - it is OK to replace it with a single \"it succeeds without\n>    unsetting the global config\".\nAt this point, I have the feeling that you have misread the diff\n(do correct me If I'm wrong) and saw ...\n>  (\n>      cd downstream &&\n>      git config fetch.recurseSubmodules on-demand &&\n>      git fetch >../actual.out 2>../actual.err\n>  ) &&\n>  git config --global --unset fetch.recurseSubmodules &&\n>  (\n>      cd downstream &&\n>      git fetch >../actual.out 2>../actual.err\n>      git config --unset fetch.recurseSubmodules\n>  ) &&\n(note the second extra git-fetch)\n\n... instead of\n\n>  429     (\n>  430         cd downstream &&\n>  431         git config fetch.recurseSubmodules on-demand &&\n>  432         git fetch >../actual.out 2>../actual.err\n>  433     ) &&\n>  434     git config --global --unset fetch.recurseSubmodules &&\n>  435     (\n>  436         cd downstream &&\n>  437         git config --unset fetch.recurseSubmodules\n>  438     ) &&\n\nIn this case, what you said (e.g. simplification etc.) makes perfect sense.\n\nThanks,\n\nFirmin\n"},{"id":"424705","messageId":"xmqqsg2l3l38.fsf@gitster.g","threadId":"55687","inReplyTo":"87fsylgadx.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","subject":"Re: [PATCH v2 2/2] t: use test_config whenever possible","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-17T06:55:55Z","receivedAt":"2021-05-17T06:56:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Firmin Martin <firminmartin24@gmail.com> writes:\n\n> fact, I suspect that you misread the diff, cf. the end of the email). \n> I copy here the full test with line number to make sure that we are in\n> the same page.\n\nThe following is the original test before your patch, right?\n\n>>  415 test_expect_success \"'fetch.recurseSubmodules=on-demand' overrides global config\" '\n>>  416     (\n>>  417         cd downstream &&\n>>  418         git fetch --recurse-submodules\n>>  419     ) &&\n>>  420     add_upstream_commit &&\n>>  421     git config --global fetch.recurseSubmodules false &&\n>>  422     head1=$(git rev-parse --short HEAD) &&\n>>  423     git add submodule &&\n>>  424     git commit -m \"new submodule\" &&\n>>  425     head2=$(git rev-parse --short HEAD) &&\n>>  426     echo \"From $pwd/.\" > expect.err.2 &&\n>>  427     echo \"   $head1..$head2  super      -> origin/super\" >>expect.err.2 &&\n>>  428     head -3 expect.err >> expect.err.2 &&\n>>  429     (\n>>  430         cd downstream &&\n>>  431         git config fetch.recurseSubmodules on-demand &&\n>>  432         git fetch >../actual.out 2>../actual.err\n\nHere we run one \"fetch\" inside downstream, with a config specific to\nthe downstream repository; the expectation is that this on-demand\nsetting is honored, overriding the \"global\" setting that was set on\nline 421.\n\n>>  433     ) &&\n>>  434     git config --global --unset fetch.recurseSubmodules &&\n\nAnd then we discard the \"global\" setting.\n\n>>  435     (\n>>  436         cd downstream &&\n>>  437         git config --unset fetch.recurseSubmodules\n\nOh, I thought I saw there was another fetch before this unset\nbetween 436 and 437, that writes to the same actual.out and\nactual.err.\n\n> At this point, I have the feeling that you have misread the diff\n> (do correct me If I'm wrong) and saw ...\n>>  (\n>>      cd downstream &&\n>>      git config fetch.recurseSubmodules on-demand &&\n>>      git fetch >../actual.out 2>../actual.err\n>>  ) &&\n>>  git config --global --unset fetch.recurseSubmodules &&\n>>  (\n>>      cd downstream &&\n>>      git fetch >../actual.out 2>../actual.err\n>>      git config --unset fetch.recurseSubmodules\n>>  ) &&\n\nExactly.  Thanks and sorry for the noise.\n"},{"id":"424709","messageId":"87cztpg5yh.fsf@Inspiron.i-did-not-set--mail-host-address--so-tickle-me","threadId":"55687","inReplyTo":"5faaa5a6-8ebf-b2ef-805f-551a36ab7e6c@gmail.com","subject":"Re: [PATCH v2 1/2] t/README: document test_config","fromName":"Firmin Martin","fromEmail":"firminmartin24@gmail.com","sentAt":"2021-05-17T07:44:22Z","receivedAt":"2021-05-17T07:44:43Z","isPatch":true,"sender":{"key":"firminmartin24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20540328?v=4"},"body":"Hi Bagas,\n\nBagas Sanjaya <bagasdotme@gmail.com> writes:\n\n> On 15/05/21 22.27, Firmin Martin wrote:\n>>   \n>> + - test_config <config-option> [<value>]\n>> +\n>> +   Set the configuration option <config-option> to <value>, and unset it at the\n>> +   end of the current test. For a similar purpose, test_config_global for\n>> +   global configuration is also available. Note, however, that test_config_*\n>> +   must not be used in a subshell.\n>> +\n\n>  From the syntax above, when I omit <value>, default value for <config-option>\n> will be set, right? \nGood remark. I thought the same as you, but no, it will \"git config\n<config-option>\" (query the value of <config-option>) and then \"git\nconfig --unset <config-option>\" which is dangerous. Doing so is more\nlikely an error and should be forbidden, as in this case, it is\nequivalent to test_unconfig <config-option>. Will reflect this in v3.\n> You forgot to mention it.\nI'll thus drop square brackets.\n\n>> +   Example:\n>> +\n>> +\ttest_config format.coverLetter auto\n>> +\n>> +   Is a concise way to write:\n>> +\ttest_when_finished \"git config --unset format.coverLetter\" &&\n>> +\tgit config format.coverLetter auto\n>> +\n>>   \n>\n> The description said \"set the config, then unset it\". But the expanded version\n> said \"unsetting the config is deferred to the end of test and set the config\".\n>\n> The documentation for test_when_finished said:\n>\n>>    Prepend <script> to a list of commands to run to clean up\n>>    at the end of the current test.  If some clean-up command\n>>    fails, the test will not pass.\n>\n> Is my interpretation above correct?\nYes.\n\n> Oh yeah, maybe it's better to say:\n> \"For example: <blah> have the same effect as <blah>.\"\nI agree, because the two lines have not exactly the same behaviour as\ntest_config. Will fix wording.\n\nThanks,\n\nFirmin\n"}]}