{"thread":{"id":"54298","subject":"[PATCH 01/11] t7001: convert tests from the old style to the current style","startedAt":"2020-09-25T17:03:17Z","lastAt":"2020-12-22T19:23:27Z","messageCount":27,"participants":["shubham verma","Eric Sunshine","Junio C Hamano","Shubham Verma"],"isPatch":true,"patchVersion":1,"patchTotal":11},"messages":[{"id":"406351","messageId":"20200925170256.11490-2-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 01/11] t7001: convert tests from the old style to the current style","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:46Z","receivedAt":"2020-09-25T17:03:17Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nTo modernize the t7001 test script, let's change the style of\nits tests from an old one to the modern one.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 192 +++++++++++++++++++++++++-------------------------\n 1 file changed, 96 insertions(+), 96 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 63d5f41a12..4bbb51e578 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -3,74 +3,74 @@\n test_description='git mv in subdirs'\n . ./test-lib.sh\n \n-test_expect_success \\\n-    'prepare reference tree' \\\n-    'mkdir path0 path1 &&\n+test_expect_success 'prepare reference tree' '\n+     mkdir path0 path1 &&\n      cp \"$TEST_DIRECTORY\"/../COPYING path0/COPYING &&\n      git add path0/COPYING &&\n-     git commit -m add -a'\n+     git commit -m add -a\n+'\n \n-test_expect_success \\\n-    'moving the file out of subdirectory' \\\n-    'cd path0 && git mv COPYING ../path1/COPYING'\n+test_expect_success 'moving the file out of subdirectory' '\n+     cd path0 && git mv COPYING ../path1/COPYING\n+'\n \n # in path0 currently\n-test_expect_success \\\n-    'commiting the change' \\\n-    'cd .. && git commit -m move-out -a'\n+test_expect_success 'commiting the change' '\n+     cd .. && git commit -m move-out -a\n+'\n \n-test_expect_success \\\n-    'checking the commit' \\\n-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-    grep \"^R100..*path0/COPYING..*path1/COPYING\" actual'\n+test_expect_success 'checking the commit' '\n+     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+     grep \"^R100..*path0/COPYING..*path1/COPYING\" actual\n+'\n \n-test_expect_success \\\n-    'moving the file back into subdirectory' \\\n-    'cd path0 && git mv ../path1/COPYING COPYING'\n+test_expect_success 'moving the file back into subdirectory' '\n+     cd path0 && git mv ../path1/COPYING COPYING\n+'\n \n # in path0 currently\n-test_expect_success \\\n-    'commiting the change' \\\n-    'cd .. && git commit -m move-in -a'\n-\n-test_expect_success \\\n-    'checking the commit' \\\n-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-    grep \"^R100..*path1/COPYING..*path0/COPYING\" actual'\n-\n-test_expect_success \\\n-    'mv --dry-run does not move file' \\\n-    'git mv -n path0/COPYING MOVED &&\n+test_expect_success 'commiting the change' '\n+     cd .. && git commit -m move-in -a\n+'\n+\n+test_expect_success 'checking the commit' '\n+     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+     grep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n+'\n+\n+test_expect_success 'mv --dry-run does not move file' '\n+     git mv -n path0/COPYING MOVED &&\n      test -f path0/COPYING &&\n-     test ! -f MOVED'\n+     test ! -f MOVED\n+'\n \n-test_expect_success \\\n-    'checking -k on non-existing file' \\\n-    'git mv -k idontexist path0'\n+test_expect_success 'checking -k on non-existing file' '\n+     git mv -k idontexist path0\n+'\n \n-test_expect_success \\\n-    'checking -k on untracked file' \\\n-    'touch untracked1 &&\n+test_expect_success 'checking -k on untracked file' '\n+     touch untracked1 &&\n      git mv -k untracked1 path0 &&\n      test -f untracked1 &&\n-     test ! -f path0/untracked1'\n+     test ! -f path0/untracked1\n+'\n \n-test_expect_success \\\n-    'checking -k on multiple untracked files' \\\n-    'touch untracked2 &&\n+test_expect_success 'checking -k on multiple untracked files' '\n+     touch untracked2 &&\n      git mv -k untracked1 untracked2 path0 &&\n      test -f untracked1 &&\n      test -f untracked2 &&\n      test ! -f path0/untracked1 &&\n-     test ! -f path0/untracked2'\n+     test ! -f path0/untracked2\n+'\n \n-test_expect_success \\\n-    'checking -f on untracked file with existing target' \\\n-    'touch path0/untracked1 &&\n+test_expect_success 'checking -f on untracked file with existing target' '\n+     touch path0/untracked1 &&\n      test_must_fail git mv -f untracked1 path0 &&\n      test ! -f .git/index.lock &&\n      test -f untracked1 &&\n-     test -f path0/untracked1'\n+     test -f path0/untracked1\n+'\n \n # clean up the mess in case bad things happen\n rm -f idontexist untracked1 untracked2 \\\n@@ -78,79 +78,79 @@ rm -f idontexist untracked1 untracked2 \\\n      .git/index.lock\n rmdir path1\n \n-test_expect_success \\\n-    'moving to absent target with trailing slash' \\\n-    'test_must_fail git mv path0/COPYING no-such-dir/ &&\n+test_expect_success 'moving to absent target with trailing slash' '\n+     test_must_fail git mv path0/COPYING no-such-dir/ &&\n      test_must_fail git mv path0/COPYING no-such-dir// &&\n      git mv path0/ no-such-dir/ &&\n-     test_path_is_dir no-such-dir'\n+     test_path_is_dir no-such-dir\n+'\n \n-test_expect_success \\\n-    'clean up' \\\n-    'git reset --hard'\n+test_expect_success 'clean up' '\n+     git reset --hard\n+'\n \n-test_expect_success \\\n-    'moving to existing untracked target with trailing slash' \\\n-    'mkdir path1 &&\n+test_expect_success 'moving to existing untracked target with trailing slash' '\n+     mkdir path1 &&\n      git mv path0/ path1/ &&\n-     test_path_is_dir path1/path0/'\n+     test_path_is_dir path1/path0/\n+'\n \n-test_expect_success \\\n-    'moving to existing tracked target with trailing slash' \\\n-    'mkdir path2 &&\n+test_expect_success 'moving to existing tracked target with trailing slash' '\n+     mkdir path2 &&\n      >path2/file && git add path2/file &&\n      git mv path1/path0/ path2/ &&\n-     test_path_is_dir path2/path0/'\n+     test_path_is_dir path2/path0/\n+'\n \n-test_expect_success \\\n-    'clean up' \\\n-    'git reset --hard'\n+test_expect_success 'clean up' '\n+     git reset --hard\n+'\n \n-test_expect_success \\\n-    'adding another file' \\\n-    'cp \"$TEST_DIRECTORY\"/../README.md path0/README &&\n+test_expect_success 'adding another file' '\n+     cp \"$TEST_DIRECTORY\"/../README.md path0/README &&\n      git add path0/README &&\n-     git commit -m add2 -a'\n+     git commit -m add2 -a\n+'\n \n-test_expect_success \\\n-    'moving whole subdirectory' \\\n-    'git mv path0 path2'\n+test_expect_success 'moving whole subdirectory' '\n+     git mv path0 path2\n+'\n \n-test_expect_success \\\n-    'commiting the change' \\\n-    'git commit -m dir-move -a'\n+test_expect_success 'commiting the change' '\n+     git commit -m dir-move -a\n+'\n \n-test_expect_success \\\n-    'checking the commit' \\\n-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+test_expect_success 'checking the commit' '\n+     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n      grep \"^R100..*path0/COPYING..*path2/COPYING\" actual &&\n-     grep \"^R100..*path0/README..*path2/README\" actual'\n+     grep \"^R100..*path0/README..*path2/README\" actual\n+'\n \n-test_expect_success \\\n-    'succeed when source is a prefix of destination' \\\n-    'git mv path2/COPYING path2/COPYING-renamed'\n+test_expect_success 'succeed when source is a prefix of destination' '\n+     git mv path2/COPYING path2/COPYING-renamed\n+'\n \n-test_expect_success \\\n-    'moving whole subdirectory into subdirectory' \\\n-    'git mv path2 path1'\n+test_expect_success 'moving whole subdirectory into subdirectory' '\n+     git mv path2 path1\n+'\n \n-test_expect_success \\\n-    'commiting the change' \\\n-    'git commit -m dir-move -a'\n+test_expect_success 'commiting the change' '\n+     git commit -m dir-move -a\n+'\n \n-test_expect_success \\\n-    'checking the commit' \\\n-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+test_expect_success 'checking the commit' '\n+     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n      grep \"^R100..*path2/COPYING..*path1/path2/COPYING\" actual &&\n-     grep \"^R100..*path2/README..*path1/path2/README\" actual'\n+     grep \"^R100..*path2/README..*path1/path2/README\" actual\n+'\n \n-test_expect_success \\\n-    'do not move directory over existing directory' \\\n-    'mkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0'\n+test_expect_success 'do not move directory over existing directory' '\n+     mkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0\n+'\n \n-test_expect_success \\\n-    'move into \".\"' \\\n-    'git mv path1/path2/ .'\n+test_expect_success 'move into \".\"' '\n+     git mv path1/path2/ .\n+'\n \n test_expect_success \"Michael Cassar's test case\" '\n \trm -fr .git papers partA &&\n-- \n2.25.1\n\n"},{"id":"406352","messageId":"20200925170256.11490-1-shubhunic@gmail.com","threadId":"54298","inReplyTo":null,"subject":"[PATCH 00/11] Modernizing the t7001 test script","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:45Z","receivedAt":"2020-09-25T17:03:19Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"In this patch series modernize the t7001 test script by changing the\nstyle of its tests from an old one to the modern one and by cleaning\nup the test script.\n\nShubham Verma (11):\n  t7001: convert tests from the old style to the current style\n  t7001: use TAB instead of spaces\n  t7001: remove unnecessary blank lines\n  t7001: change the style for cd according to subshell\n  t7001: remove whitespace after redirect operators\n  t7001: change (cd <path> && git foo) to (git -C <path> foo)\n  t7001: use ': >' rather than 'touch'\n  t7001: put each command on a separate line\n  t7001: use here-docs instead of echo\n  t7001: use `test` rather than `[`\n  t7001: move cleanup code from outside the tests into them\n\n t/t7001-mv.sh | 393 +++++++++++++++++++++++---------------------------\n 1 file changed, 181 insertions(+), 212 deletions(-)\n\n-- \n2.25.1\n\n"},{"id":"406353","messageId":"20200925170256.11490-3-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 02/11] t7001: use TAB instead of spaces","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:47Z","receivedAt":"2020-09-25T17:03:20Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nChange indentation style from spaces to TAB.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 120 +++++++++++++++++++++++++-------------------------\n 1 file changed, 60 insertions(+), 60 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 4bbb51e578..7503233814 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -4,72 +4,72 @@ test_description='git mv in subdirs'\n . ./test-lib.sh\n \n test_expect_success 'prepare reference tree' '\n-     mkdir path0 path1 &&\n-     cp \"$TEST_DIRECTORY\"/../COPYING path0/COPYING &&\n-     git add path0/COPYING &&\n-     git commit -m add -a\n+\tmkdir path0 path1 &&\n+\tcp \"$TEST_DIRECTORY\"/../COPYING path0/COPYING &&\n+\tgit add path0/COPYING &&\n+\tgit commit -m add -a\n '\n \n test_expect_success 'moving the file out of subdirectory' '\n-     cd path0 && git mv COPYING ../path1/COPYING\n+\tcd path0 && git mv COPYING ../path1/COPYING\n '\n \n # in path0 currently\n test_expect_success 'commiting the change' '\n-     cd .. && git commit -m move-out -a\n+\tcd .. && git commit -m move-out -a\n '\n \n test_expect_success 'checking the commit' '\n-     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-     grep \"^R100..*path0/COPYING..*path1/COPYING\" actual\n+\tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+\tgrep \"^R100..*path0/COPYING..*path1/COPYING\" actual\n '\n \n test_expect_success 'moving the file back into subdirectory' '\n-     cd path0 && git mv ../path1/COPYING COPYING\n+\tcd path0 && git mv ../path1/COPYING COPYING\n '\n \n # in path0 currently\n test_expect_success 'commiting the change' '\n-     cd .. && git commit -m move-in -a\n+\tcd .. && git commit -m move-in -a\n '\n \n test_expect_success 'checking the commit' '\n-     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-     grep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n+\tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+\tgrep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n '\n \n test_expect_success 'mv --dry-run does not move file' '\n-     git mv -n path0/COPYING MOVED &&\n-     test -f path0/COPYING &&\n-     test ! -f MOVED\n+\tgit mv -n path0/COPYING MOVED &&\n+\ttest -f path0/COPYING &&\n+\ttest ! -f MOVED\n '\n \n test_expect_success 'checking -k on non-existing file' '\n-     git mv -k idontexist path0\n+\tgit mv -k idontexist path0\n '\n \n test_expect_success 'checking -k on untracked file' '\n-     touch untracked1 &&\n-     git mv -k untracked1 path0 &&\n-     test -f untracked1 &&\n-     test ! -f path0/untracked1\n+\ttouch untracked1 &&\n+\tgit mv -k untracked1 path0 &&\n+\ttest -f untracked1 &&\n+\ttest ! -f path0/untracked1\n '\n \n test_expect_success 'checking -k on multiple untracked files' '\n-     touch untracked2 &&\n-     git mv -k untracked1 untracked2 path0 &&\n-     test -f untracked1 &&\n-     test -f untracked2 &&\n-     test ! -f path0/untracked1 &&\n-     test ! -f path0/untracked2\n+\ttouch untracked2 &&\n+\tgit mv -k untracked1 untracked2 path0 &&\n+\ttest -f untracked1 &&\n+\ttest -f untracked2 &&\n+\ttest ! -f path0/untracked1 &&\n+\ttest ! -f path0/untracked2\n '\n \n test_expect_success 'checking -f on untracked file with existing target' '\n-     touch path0/untracked1 &&\n-     test_must_fail git mv -f untracked1 path0 &&\n-     test ! -f .git/index.lock &&\n-     test -f untracked1 &&\n-     test -f path0/untracked1\n+\ttouch path0/untracked1 &&\n+\ttest_must_fail git mv -f untracked1 path0 &&\n+\ttest ! -f .git/index.lock &&\n+\ttest -f untracked1 &&\n+\ttest -f path0/untracked1\n '\n \n # clean up the mess in case bad things happen\n@@ -79,77 +79,77 @@ rm -f idontexist untracked1 untracked2 \\\n rmdir path1\n \n test_expect_success 'moving to absent target with trailing slash' '\n-     test_must_fail git mv path0/COPYING no-such-dir/ &&\n-     test_must_fail git mv path0/COPYING no-such-dir// &&\n-     git mv path0/ no-such-dir/ &&\n-     test_path_is_dir no-such-dir\n+\ttest_must_fail git mv path0/COPYING no-such-dir/ &&\n+\ttest_must_fail git mv path0/COPYING no-such-dir// &&\n+\tgit mv path0/ no-such-dir/ &&\n+\ttest_path_is_dir no-such-dir\n '\n \n test_expect_success 'clean up' '\n-     git reset --hard\n+\tgit reset --hard\n '\n \n test_expect_success 'moving to existing untracked target with trailing slash' '\n-     mkdir path1 &&\n-     git mv path0/ path1/ &&\n-     test_path_is_dir path1/path0/\n+\tmkdir path1 &&\n+\tgit mv path0/ path1/ &&\n+\ttest_path_is_dir path1/path0/\n '\n \n test_expect_success 'moving to existing tracked target with trailing slash' '\n-     mkdir path2 &&\n-     >path2/file && git add path2/file &&\n-     git mv path1/path0/ path2/ &&\n-     test_path_is_dir path2/path0/\n+\tmkdir path2 &&\n+\t>path2/file && git add path2/file &&\n+\tgit mv path1/path0/ path2/ &&\n+\ttest_path_is_dir path2/path0/\n '\n \n test_expect_success 'clean up' '\n-     git reset --hard\n+\tgit reset --hard\n '\n \n test_expect_success 'adding another file' '\n-     cp \"$TEST_DIRECTORY\"/../README.md path0/README &&\n-     git add path0/README &&\n-     git commit -m add2 -a\n+\tcp \"$TEST_DIRECTORY\"/../README.md path0/README &&\n+\tgit add path0/README &&\n+\tgit commit -m add2 -a\n '\n \n test_expect_success 'moving whole subdirectory' '\n-     git mv path0 path2\n+\tgit mv path0 path2\n '\n \n test_expect_success 'commiting the change' '\n-     git commit -m dir-move -a\n+\tgit commit -m dir-move -a\n '\n \n test_expect_success 'checking the commit' '\n-     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-     grep \"^R100..*path0/COPYING..*path2/COPYING\" actual &&\n-     grep \"^R100..*path0/README..*path2/README\" actual\n+\tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+\tgrep \"^R100..*path0/COPYING..*path2/COPYING\" actual &&\n+\tgrep \"^R100..*path0/README..*path2/README\" actual\n '\n \n test_expect_success 'succeed when source is a prefix of destination' '\n-     git mv path2/COPYING path2/COPYING-renamed\n+\tgit mv path2/COPYING path2/COPYING-renamed\n '\n \n test_expect_success 'moving whole subdirectory into subdirectory' '\n-     git mv path2 path1\n+\tgit mv path2 path1\n '\n \n test_expect_success 'commiting the change' '\n-     git commit -m dir-move -a\n+\tgit commit -m dir-move -a\n '\n \n test_expect_success 'checking the commit' '\n-     git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n-     grep \"^R100..*path2/COPYING..*path1/path2/COPYING\" actual &&\n-     grep \"^R100..*path2/README..*path1/path2/README\" actual\n+\tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n+\tgrep \"^R100..*path2/COPYING..*path1/path2/COPYING\" actual &&\n+\tgrep \"^R100..*path2/README..*path1/path2/README\" actual\n '\n \n test_expect_success 'do not move directory over existing directory' '\n-     mkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0\n+\tmkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0\n '\n \n test_expect_success 'move into \".\"' '\n-     git mv path1/path2/ .\n+\tgit mv path1/path2/ .\n '\n \n test_expect_success \"Michael Cassar's test case\" '\n-- \n2.25.1\n\n"},{"id":"406354","messageId":"20200925170256.11490-4-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 03/11] t7001: remove unnecessary blank lines","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:48Z","receivedAt":"2020-09-25T17:03:22Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nSome tests use a deprecated style in which there are unnecessary\nblank lines after the opening quote of the test body and before the\nclosing quote. So we should removed these unnecessary blank lines.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 12 ------------\n 1 file changed, 12 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 7503233814..f63802442b 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -182,7 +182,6 @@ test_expect_success \"Sergey Vlasov's test case\" '\n '\n \n test_expect_success 'absolute pathname' '(\n-\n \trm -fr mine &&\n \tmkdir mine &&\n \tcd mine &&\n@@ -196,12 +195,9 @@ test_expect_success 'absolute pathname' '(\n \t! test -d sub &&\n \ttest -d in &&\n \tgit ls-files --error-unmatch in/file\n-\n-\n )'\n \n test_expect_success 'absolute pathname outside should fail' '(\n-\n \trm -fr mine &&\n \tmkdir mine &&\n \tcd mine &&\n@@ -216,7 +212,6 @@ test_expect_success 'absolute pathname outside should fail' '(\n \ttest -d sub &&\n \t! test -d ../in &&\n \tgit ls-files --error-unmatch sub/file\n-\n )'\n \n test_expect_success 'git mv to move multiple sources into a directory' '\n@@ -232,7 +227,6 @@ test_expect_success 'git mv to move multiple sources into a directory' '\n '\n \n test_expect_success 'git mv should not change sha1 of moved cache entry' '\n-\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >dirty &&\n@@ -243,7 +237,6 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '\n \techo 2 >dirty2 &&\n \tgit mv dirty2 dirty &&\n \t[ \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\" ]\n-\n '\n \n rm -f dirty dirty2\n@@ -266,7 +259,6 @@ test_expect_success 'git mv error on conflicted file' '\n '\n \n test_expect_success 'git mv should overwrite symlink to a file' '\n-\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >moved &&\n@@ -279,13 +271,11 @@ test_expect_success 'git mv should overwrite symlink to a file' '\n \ttest \"$(cat symlink)\" = 1 &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n-\n '\n \n rm -f moved symlink\n \n test_expect_success 'git mv should overwrite file with a symlink' '\n-\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >moved &&\n@@ -296,11 +286,9 @@ test_expect_success 'git mv should overwrite file with a symlink' '\n \t! test -e symlink &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n-\n '\n \n test_expect_success SYMLINKS 'check moved symlink' '\n-\n \ttest -h moved\n '\n \n-- \n2.25.1\n\n"},{"id":"406355","messageId":"20200925170256.11490-5-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 04/11] t7001: change the style for cd according to subshell","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:49Z","receivedAt":"2020-09-25T17:03:25Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nIn some tests there is not a proper spaces after opening paranthesis\nand before cd. So, Lets change the style for cd according to subshell.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex f63802442b..67585b7d94 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -503,14 +503,16 @@ test_expect_success 'moving a submodule in nested directories' '\n test_expect_success 'moving nested submodules' '\n \tgit commit -am \"cleanup commit\" &&\n \tmkdir sub_nested_nested &&\n-\t(cd sub_nested_nested &&\n+\t(\n+\t\tcd sub_nested_nested &&\n \t\ttouch nested_level2 &&\n \t\tgit init &&\n \t\tgit add . &&\n \t\tgit commit -m \"nested level 2\"\n \t) &&\n \tmkdir sub_nested &&\n-\t(cd sub_nested &&\n+\t(\n+\t\tcd sub_nested &&\n \t\ttouch nested_level1 &&\n \t\tgit init &&\n \t\tgit add . &&\n-- \n2.25.1\n\n"},{"id":"406356","messageId":"20200925170256.11490-6-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 05/11] t7001: remove whitespace after redirect operators","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:50Z","receivedAt":"2020-09-25T17:03:28Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nAccording to Documentation/CodingGuidelines, there should be no\nwhitespace after redirect operators. So, we should remove these\nwhitespaces after redirect operators.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 67585b7d94..7581e4b407 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -156,9 +156,9 @@ test_expect_success \"Michael Cassar's test case\" '\n \trm -fr .git papers partA &&\n \tgit init &&\n \tmkdir -p papers/unsorted papers/all-papers partA &&\n-\techo a > papers/unsorted/Thesis.pdf &&\n-\techo b > partA/outline.txt &&\n-\techo c > papers/unsorted/_another &&\n+\techo a >papers/unsorted/Thesis.pdf &&\n+\techo b >partA/outline.txt &&\n+\techo c >papers/unsorted/_another &&\n \tgit add papers partA &&\n \tT1=$(git write-tree) &&\n \n-- \n2.25.1\n\n"},{"id":"406357","messageId":"20200925170256.11490-7-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 06/11] t7001: change (cd <path> && git foo) to (git -C <path> foo)","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:51Z","receivedAt":"2020-09-25T17:03:30Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nLet's avoid the use of `cd` outside subshells by encapsulating them\ninside subshells or by using `git -C <dir> ...`.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 45 +++++++++++----------------------------------\n 1 file changed, 11 insertions(+), 34 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 7581e4b407..3a3ace6d73 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -11,12 +11,11 @@ test_expect_success 'prepare reference tree' '\n '\n \n test_expect_success 'moving the file out of subdirectory' '\n-\tcd path0 && git mv COPYING ../path1/COPYING\n+\tgit -C path0 mv COPYING ../path1/COPYING\n '\n \n-# in path0 currently\n test_expect_success 'commiting the change' '\n-\tcd .. && git commit -m move-out -a\n+\tgit commit -m move-out -a\n '\n \n test_expect_success 'checking the commit' '\n@@ -25,12 +24,11 @@ test_expect_success 'checking the commit' '\n '\n \n test_expect_success 'moving the file back into subdirectory' '\n-\tcd path0 && git mv ../path1/COPYING COPYING\n+\tgit -C path0 mv ../path1/COPYING COPYING\n '\n \n-# in path0 currently\n test_expect_success 'commiting the change' '\n-\tcd .. && git commit -m move-in -a\n+\tgit commit -m move-in -a\n '\n \n test_expect_success 'checking the commit' '\n@@ -324,10 +322,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm\n \tgit mv sub mod/sub &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n '\n@@ -347,10 +342,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu\n \tgit mv sub mod/sub &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n \ttest_cmp expected actual &&\n@@ -364,16 +356,10 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n \tgit submodule update &&\n \tentry=\"$(git ls-files --stage sub | cut -f 1)\" &&\n \tmkdir mod &&\n-\t(\n-\t\tcd mod &&\n-\t\tgit mv ../sub/ .\n-\t) &&\n+\tgit -C mod mv ../sub/ . &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n \ttest_cmp expected actual &&\n@@ -392,10 +378,7 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '\n \ttest_must_be_empty actual.err &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n '\n@@ -416,10 +399,7 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta\n \ttest_must_be_empty actual.err &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n '\n@@ -437,10 +417,7 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule\n \ttest_i18ncmp expect.err actual.err &&\n \t! test -e sub &&\n \t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n-\t(\n-\t\tcd mod/sub &&\n-\t\tgit status\n-\t) &&\n+\tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n '\n-- \n2.25.1\n\n"},{"id":"406358","messageId":"20200925170256.11490-8-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 07/11] t7001: use ': >' rather than 'touch'","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:52Z","receivedAt":"2020-09-25T17:03:32Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nUse `>` rather than `touch` to create an empty file when the\ntimestamp isn't relevant to the test.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 3a3ace6d73..728a937eeb 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -47,14 +47,14 @@ test_expect_success 'checking -k on non-existing file' '\n '\n \n test_expect_success 'checking -k on untracked file' '\n-\ttouch untracked1 &&\n+\t: > untracked1 &&\n \tgit mv -k untracked1 path0 &&\n \ttest -f untracked1 &&\n \ttest ! -f path0/untracked1\n '\n \n test_expect_success 'checking -k on multiple untracked files' '\n-\ttouch untracked2 &&\n+\t: > untracked2 &&\n \tgit mv -k untracked1 untracked2 path0 &&\n \ttest -f untracked1 &&\n \ttest -f untracked2 &&\n@@ -63,7 +63,7 @@ test_expect_success 'checking -k on multiple untracked files' '\n '\n \n test_expect_success 'checking -f on untracked file with existing target' '\n-\ttouch path0/untracked1 &&\n+\t: > path0/untracked1 &&\n \ttest_must_fail git mv -f untracked1 path0 &&\n \ttest ! -f .git/index.lock &&\n \ttest -f untracked1 &&\n@@ -482,7 +482,7 @@ test_expect_success 'moving nested submodules' '\n \tmkdir sub_nested_nested &&\n \t(\n \t\tcd sub_nested_nested &&\n-\t\ttouch nested_level2 &&\n+\t\t: > nested_level2 &&\n \t\tgit init &&\n \t\tgit add . &&\n \t\tgit commit -m \"nested level 2\"\n@@ -490,7 +490,7 @@ test_expect_success 'moving nested submodules' '\n \tmkdir sub_nested &&\n \t(\n \t\tcd sub_nested &&\n-\t\ttouch nested_level1 &&\n+\t\t: > nested_level1 &&\n \t\tgit init &&\n \t\tgit add . &&\n \t\tgit commit -m \"nested level 1\" &&\n-- \n2.25.1\n\n"},{"id":"406359","messageId":"20200925170256.11490-9-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 08/11] t7001: put each command on a separate line","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:53Z","receivedAt":"2020-09-25T17:03:35Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nMultiple commands on one line  should be split across multiple lines.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 728a937eeb..94c5b10f8a 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -143,7 +143,9 @@ test_expect_success 'checking the commit' '\n '\n \n test_expect_success 'do not move directory over existing directory' '\n-\tmkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0\n+\tmkdir path0 &&\n+\tmkdir path0/path2 &&\n+\ttest_must_fail git mv path2 path0\n '\n \n test_expect_success 'move into \".\"' '\n-- \n2.25.1\n\n"},{"id":"406360","messageId":"20200925170256.11490-10-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 09/11] t7001: use here-docs instead of echo","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:54Z","receivedAt":"2020-09-25T17:03:37Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nChange from old style to current style by taking advantage of\nhere-docs instead of echo commands.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 94c5b10f8a..30714a8200 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -222,7 +222,10 @@ test_expect_success 'git mv to move multiple sources into a directory' '\n \tgit add dir/?.txt &&\n \tgit mv dir/a.txt dir/b.txt other &&\n \tgit ls-files >actual &&\n-\t{ echo other/a.txt; echo other/b.txt; } >expect &&\n+\tcat >expect <<-\\EOF &&\n+\tother/a.txt\n+\tother/b.txt\n+\tEOF\n \ttest_cmp expect actual\n '\n \n-- \n2.25.1\n\n"},{"id":"406361","messageId":"20200925170256.11490-11-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 10/11] t7001: use `test` rather than `[`","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:55Z","receivedAt":"2020-09-25T17:03:40Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nAccording to Documentation/CodingGuidelines, we should use \"test\"\nrather than \"[ ... ]\" in shell scripts, so let's replace the\n\"[ ... ]\" with \"test\" in the t7001 test script.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 30714a8200..7bb4a7b759 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -236,10 +236,10 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '\n \tgit add dirty &&\n \tentry=\"$(git ls-files --stage dirty | cut -f 1)\" &&\n \tgit mv dirty dirty2 &&\n-\t[ \"$entry\" = \"$(git ls-files --stage dirty2 | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage dirty2 | cut -f 1)\" &&\n \techo 2 >dirty2 &&\n \tgit mv dirty2 dirty &&\n-\t[ \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\" ]\n+\ttest \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\"\n '\n \n rm -f dirty dirty2\n@@ -326,7 +326,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and no .gitm\n \tmkdir mod &&\n \tgit mv sub mod/sub &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -346,7 +346,7 @@ test_expect_success 'git mv moves a submodule with a .git directory and .gitmodu\n \tmkdir mod &&\n \tgit mv sub mod/sub &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n@@ -363,7 +363,7 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n \tmkdir mod &&\n \tgit -C mod mv ../sub/ . &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \techo mod/sub >expected &&\n \tgit config -f .gitmodules submodule.sub.path >actual &&\n@@ -382,7 +382,7 @@ test_expect_success 'mv does not complain when no .gitmodules file is found' '\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_must_be_empty actual.err &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -403,7 +403,7 @@ test_expect_success 'mv will error out on a modified .gitmodules file unless sta\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_must_be_empty actual.err &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n@@ -421,7 +421,7 @@ test_expect_success 'mv issues a warning when section is not found in .gitmodule\n \tgit mv sub mod/sub 2>actual.err &&\n \ttest_i18ncmp expect.err actual.err &&\n \t! test -e sub &&\n-\t[ \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" ] &&\n+\ttest \"$entry\" = \"$(git ls-files --stage mod/sub | cut -f 1)\" &&\n \tgit -C mod/sub status &&\n \tgit update-index --refresh &&\n \tgit diff-files --quiet\n-- \n2.25.1\n\n"},{"id":"406362","messageId":"20200925170256.11490-12-shubhunic@gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"[PATCH 11/11] t7001: move cleanup code from outside the tests into them","fromName":"shubham verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-09-25T17:02:56Z","receivedAt":"2020-09-25T17:03:43Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"From: Shubham Verma <shubhunic@gmail.com>\n\nLet's use test_when_finished() to include cleanup code inside the tests,\nas it's cleaner and safer to not have any code outside the tests.\n\nSigned-off-by: shubham verma <shubhunic@gmail.com>\n---\n t/t7001-mv.sh | 25 +++++++++++--------------\n 1 file changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 7bb4a7b759..b4d04ceaf8 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -32,6 +32,7 @@ test_expect_success 'commiting the change' '\n '\n \n test_expect_success 'checking the commit' '\n+\ttest_when_finished \"rmdir path1\" &&\n \tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n \tgrep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n '\n@@ -43,6 +44,7 @@ test_expect_success 'mv --dry-run does not move file' '\n '\n \n test_expect_success 'checking -k on non-existing file' '\n+\ttest_when_finished \"rm -f idontexist path0/idontexist\" &&\n \tgit mv -k idontexist path0\n '\n \n@@ -55,6 +57,7 @@ test_expect_success 'checking -k on untracked file' '\n \n test_expect_success 'checking -k on multiple untracked files' '\n \t: > untracked2 &&\n+\ttest_when_finished \"rm -f untracked2 path0/untracked2\" &&\n \tgit mv -k untracked1 untracked2 path0 &&\n \ttest -f untracked1 &&\n \ttest -f untracked2 &&\n@@ -64,18 +67,14 @@ test_expect_success 'checking -k on multiple untracked files' '\n \n test_expect_success 'checking -f on untracked file with existing target' '\n \t: > path0/untracked1 &&\n+\ttest_when_finished \"rm -f untracked1 path0/untracked1\" &&\n+\ttest_when_finished \"rm -f .git/index.lock\" &&\n \ttest_must_fail git mv -f untracked1 path0 &&\n \ttest ! -f .git/index.lock &&\n \ttest -f untracked1 &&\n \ttest -f path0/untracked1\n '\n \n-# clean up the mess in case bad things happen\n-rm -f idontexist untracked1 untracked2 \\\n-     path0/idontexist path0/untracked1 path0/untracked2 \\\n-     .git/index.lock\n-rmdir path1\n-\n test_expect_success 'moving to absent target with trailing slash' '\n \ttest_must_fail git mv path0/COPYING no-such-dir/ &&\n \ttest_must_fail git mv path0/COPYING no-such-dir// &&\n@@ -149,10 +148,12 @@ test_expect_success 'do not move directory over existing directory' '\n '\n \n test_expect_success 'move into \".\"' '\n+\ttest_when_finished \"rm -fr path?\" &&\n \tgit mv path1/path2/ .\n '\n \n test_expect_success \"Michael Cassar's test case\" '\n+\ttest_when_finished \"rm -fr papers partA\" &&\n \trm -fr .git papers partA &&\n \tgit init &&\n \tmkdir -p papers/unsorted papers/all-papers partA &&\n@@ -168,8 +169,6 @@ test_expect_success \"Michael Cassar's test case\" '\n \tgit ls-tree -r $T | verbose grep partA/outline.txt\n '\n \n-rm -fr papers partA path?\n-\n test_expect_success \"Sergey Vlasov's test case\" '\n \trm -fr .git &&\n \tgit init &&\n@@ -230,6 +229,7 @@ test_expect_success 'git mv to move multiple sources into a directory' '\n '\n \n test_expect_success 'git mv should not change sha1 of moved cache entry' '\n+\ttest_when_finished \"rm -f dirty dirty2\" &&\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >dirty &&\n@@ -242,8 +242,6 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '\n \ttest \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\"\n '\n \n-rm -f dirty dirty2\n-\n # NB: This test is about the error message\n # as well as the failure.\n test_expect_success 'git mv error on conflicted file' '\n@@ -262,6 +260,7 @@ test_expect_success 'git mv error on conflicted file' '\n '\n \n test_expect_success 'git mv should overwrite symlink to a file' '\n+\ttest_when_finished \"rm -f moved symlink\" &&\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >moved &&\n@@ -276,9 +275,8 @@ test_expect_success 'git mv should overwrite symlink to a file' '\n \tgit diff-files --quiet\n '\n \n-rm -f moved symlink\n-\n test_expect_success 'git mv should overwrite file with a symlink' '\n+\ttest_when_finished \"rm -f symlink\" &&\n \trm -fr .git &&\n \tgit init &&\n \techo 1 >moved &&\n@@ -292,11 +290,10 @@ test_expect_success 'git mv should overwrite file with a symlink' '\n '\n \n test_expect_success SYMLINKS 'check moved symlink' '\n+\ttest_when_finished \"rm -f moved\" &&\n \ttest -h moved\n '\n \n-rm -f moved symlink\n-\n test_expect_success 'setup submodule' '\n \tgit commit -m initial &&\n \tgit reset --hard &&\n-- \n2.25.1\n\n"},{"id":"406366","messageId":"CAPig+cTwHuTA=puzPfpOiMcMUkYPsbOca1GQK=dS1hJJeZZBGg@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-1-shubhunic@gmail.com","subject":"Re: [PATCH 00/11] Modernizing the t7001 test script","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T17:33:15Z","receivedAt":"2020-09-25T17:33:29Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> In this patch series modernize the t7001 test script by changing the\n> style of its tests from an old one to the modern one and by cleaning\n> up the test script.\n\nThanks for tackling this task. I presume it was prompted by [1] or\n[2], as this series covers many of the items mentioned in [1].\nOverall, the series looks good. I'll leave comments in a few of the\nindividual patches.\n\n[1]: https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/\n[2]: https://git.github.io/rev_news/2020/08/27/edition-66/\n"},{"id":"406367","messageId":"CAPig+cQz5GjWH8PXkTsZ_cygmjjaxLBhRrZd_k2k23-aznRxhg@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-2-shubhunic@gmail.com","subject":"Re: [PATCH 01/11] t7001: convert tests from the old style to the current style","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T17:40:47Z","receivedAt":"2020-09-25T17:41:01Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> t7001: convert tests from the old style to the current style\n>\n> To modernize the t7001 test script, let's change the style of\n> its tests from an old one to the modern one.\n\nThis commit message could help reviewers more by giving an example of\nhow the style is being updated because it's not so easy to pick up the\ndetails from the patch itself since it's so noisy. Perhaps the commit\nmessage could say something like:\n\n    t7001: modernize test formatting\n\n    Some tests in this script are formatted using a very old style:\n\n        test_expect_success \\\n            'title' \\\n            'body line 1 &&\n            body line 2'\n\n    Update the formatting to the modern style:\n\n        test_expect_success 'title' '\n            body line 1 &&\n            body line 2\n        '\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n"},{"id":"406368","messageId":"CAPig+cSr_EY7Z1tqEZyWF6W1Bxs8aOWyypv6m4Gv0ACz=j+O4Q@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-3-shubhunic@gmail.com","subject":"Re: [PATCH 02/11] t7001: use TAB instead of spaces","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T17:44:09Z","receivedAt":"2020-09-25T17:44:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> t7001: use TAB instead of spaces\n\nNit: \"use\" is a bit nebulous; I'd probably say:\n\n    t7001: indent with TABs instead of spaces\n\n> Change indentation style from spaces to TAB.\n\n...which would also allow you to drop this line altogether.\n\nIf you feel that you must write something in the body of the commit\nmessages, then perhaps say something about how modern style is to\nindent test bodies with TABs rather than spaces.\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n"},{"id":"406370","messageId":"CAPig+cQWvn0YXUZRHE2ocH8+FS1aAnhjKiMsQvXXkNQ7LgEmfg@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-4-shubhunic@gmail.com","subject":"Re: [PATCH 03/11] t7001: remove unnecessary blank lines","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T17:50:09Z","receivedAt":"2020-09-25T17:50:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> Some tests use a deprecated style in which there are unnecessary\n> blank lines after the opening quote of the test body and before the\n> closing quote. So we should removed these unnecessary blank lines.\n\ns/So we should removed/Remove/\n\nOtherwise, nice explanatory commit message.\n\nOne comment below not related directly to this patch (just something\nnoticed in the patch conext lines)...\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> @@ -182,7 +182,6 @@ test_expect_success \"Sergey Vlasov's test case\" '\n>  test_expect_success 'absolute pathname' '(\n>      ...\n>  )'\n>  test_expect_success 'absolute pathname outside should fail' '(\n>      ...\n>  )'\n\nIt is very uncommon style to hide the subshell as these two tests do:\n\n    test_expect_success 'title' '(\n        ...\n    )'\n\nInstead, these should be formatted as:\n\n    test_expect_success 'title' '\n        (\n            ...\n        )\n    '\n\nNote that the \"(\" and \")\" of the subshell are indented with a TAB, and\nthen the body of the subshell is indented again with another TAB in\norder to comply with current style guidelines.\n\nFixing these might possibly be done in patch [1/11], however, they are\nso unusual and would change indentation of the body lines that they\nmight deserve a patch of their own to avoid being lost in the noise of\n[1/11].\n"},{"id":"406372","messageId":"CAPig+cSb2nB0cK-3Xh=5mhTa6Knh0in-wFE-DQ7MiKGWv=qR=g@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-5-shubhunic@gmail.com","subject":"Re: [PATCH 04/11] t7001: change the style for cd according to subshell","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T18:12:00Z","receivedAt":"2020-09-25T18:12:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> t7001: change the style for cd according to subshell\n>\n> In some tests there is not a proper spaces after opening paranthesis\n> and before cd. So, Lets change the style for cd according to subshell.\n\nNits:\ns/space/newline/\ns/paranthesis/parenthesis/\ns/So, Lets change/Change/\n\nHowever, a more significant observation is that this change is\nactually specific to the formatting of subshells, and has nothing to\ndo with placement of `cd`, so calling out `cd` in the commit message\nis misleading. I'd probably drop mention of `cd` altogether and write\nthe commit message something like this:\n\n    t7001: modernize subshell formatting\n\n    Some test use an old style for formatting subshells:\n\n        (command &&\n            ...\n\n    Update them to the modern style:\n\n        (\n            command &&\n            ...\n\nThe actual patch is fine.\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n"},{"id":"406380","messageId":"CAPig+cQipTK8ePVLKMeytmaePZ8yWSoFCjue=huKORjPGpTg+Q@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-7-shubhunic@gmail.com","subject":"Re: [PATCH 06/11] t7001: change (cd <path> && git foo) to (git -C <path> foo)","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T18:53:40Z","receivedAt":"2020-09-25T18:53:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> t7001: change (cd <path> && git foo) to (git -C <path> foo)\n\nThis is misleading. We don't want the `git -C` form in a subshell, so\nit shouldn't be enclosed in parentheses. Perhaps write it like this:\n\n    t7001 use `git -C` to avoid `cd` outside of subshells\n\n> Let's avoid the use of `cd` outside subshells by encapsulating them\n> inside subshells or by using `git -C <dir> ...`.\n\nThis is misleading in two ways. First, none of the changes made by\nthis patch add subshell encapsulation. Second, many of the changes\ndrop the subhsell in favor of `git -C`, so describing them as \"`cd`\noutside of subshells\" is wrong.\n\nIt's also important for the commit message to explain _why_ this\nchange is important when `cd` is used outside of a subshell. A\npossible rewrite might be:\n\n    t7001: avoid using `cd` outside of subshells\n\n    Avoid using `cd` outside of subshells since, if the test fails,\n    there is no guarantee that the current working directory is the\n    expected one, which may cause subsequent tests to run in the wrong\n    directory.\n\n    While at it, make some other tests more concise by replacing\n    simple subshells with `git -C`.\n\nIn fact, fixing the cases in which `cd` is used outside of a subshell\nis much more important than the mere mechanical conversion made to the\nother tests by replacing a subshell with `git -C`. As such, I'm\ntempted to suggest splitting this patch into two: one which fixes the\ncases of `cd` outside of subshell, and another which converts the\nsimple subshell cases to use `git -C`.\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> @@ -11,12 +11,11 @@ test_expect_success 'prepare reference tree' '\n>  test_expect_success 'moving the file out of subdirectory' '\n> -       cd path0 && git mv COPYING ../path1/COPYING\n> +       git -C path0 mv COPYING ../path1/COPYING\n>  '\n>\n> -# in path0 currently\n>  test_expect_success 'commiting the change' '\n> -       cd .. && git commit -m move-out -a\n> +       git commit -m move-out -a\n>  '\n\nThis transformation looks fine, as do the following two tests which\nget the same transformation.\n\nI do have a very slight hesitation, though, that these changes go\nagainst the grain of the tests. In particular, at the top of this\nscript, we see:\n\n    test_description='git mv in subdirs'\n\nwhich suggests that the tests really want to test the bare `git mv`\ncommand while actually running in a subdirectory. This would imply\nthat these test should be rewritten as:\n\n    test_expect_success 'title' '\n        (\n            cd path0 &&\n            ...\n        )\n    '\n\nHowever, it's such a minor misgiving that it's probably not worth considering.\n\n> @@ -364,16 +356,10 @@ test_expect_success 'git mv moves a submodule with gitfile' '\n> -       (\n> -               cd mod &&\n> -               git mv ../sub/ .\n> -       ) &&\n> +       git -C mod mv ../sub/ . &&\n\nOkay. At first glance one might expect you to strip the `../` from the\nargument, but indeed `../sub/` is correct since `-C mod` really does\nchange to the new directory, so the argument is interpreted relative\nto `mod`.\n"},{"id":"406381","messageId":"CAPig+cTwQCSA1sjAWyDC5ZydT6NKzOY9ziLUJcOVMRV4cP_sAQ@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-8-shubhunic@gmail.com","subject":"Re: [PATCH 07/11] t7001: use ': >' rather than 'touch'","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T18:57:40Z","receivedAt":"2020-09-25T18:57:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> t7001: use ': >' rather than 'touch'\n>\n> Use `>` rather than `touch` to create an empty file when the\n> timestamp isn't relevant to the test.\n\nThere is an inconsistency here. In the subject you say \": >\" but in\nthe body just \">\".\n\nA couple more comments below...\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> @@ -47,14 +47,14 @@ test_expect_success 'checking -k on non-existing file' '\n>  test_expect_success 'checking -k on untracked file' '\n> -       touch untracked1 &&\n> +       : > untracked1 &&\n\nIn patch [5/11] you dropped whitespace following the redirection\noperator, however, this patch introduces several new cases of unwanted\nwhitespace.\n\nChecking \"master\", I see that there are 209 instances of `: >` in\ntests, but 1023 instances of `>`, which suggests that we should stick\nwith plain `>` rather than `: >` in this patch.\n"},{"id":"406383","messageId":"CAPig+cTeBoGCUgo9rrqEszCJve2vZd9Jn=hUGrwxq-vaOzBE+Q@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-9-shubhunic@gmail.com","subject":"Re: [PATCH 08/11] t7001: put each command on a separate line","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T19:01:49Z","receivedAt":"2020-09-25T19:02:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> Multiple commands on one line  should be split across multiple lines.\n\nDrop the extra whitespace between \"line\" and \"should\".\n\nI might have written the commit message with a bit more explanation,\nperhaps like this:\n\n    Modern practice is to avoid multiple commands per line, and\n    instead place each command on its own line.\n\nThe patch itself looks fine.\n\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n"},{"id":"406387","messageId":"xmqqr1qpk3l3.fsf@gitster.c.googlers.com","threadId":"54298","inReplyTo":"20200925170256.11490-10-shubhunic@gmail.com","subject":"Re: [PATCH 09/11] t7001: use here-docs instead of echo","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T20:23:52Z","receivedAt":"2020-09-25T20:36:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"shubham verma <shubhunic@gmail.com> writes:\n\n> From: Shubham Verma <shubhunic@gmail.com>\n>\n> Change from old style to current style by taking advantage of\n> here-docs instead of echo commands.\n>\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n>  t/t7001-mv.sh | 5 ++++-\n>  1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index 94c5b10f8a..30714a8200 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -222,7 +222,10 @@ test_expect_success 'git mv to move multiple sources into a directory' '\n>  \tgit add dir/?.txt &&\n>  \tgit mv dir/a.txt dir/b.txt other &&\n>  \tgit ls-files >actual &&\n> -\t{ echo other/a.txt; echo other/b.txt; } >expect &&\n> +\tcat >expect <<-\\EOF &&\n> +\tother/a.txt\n> +\tother/b.txt\n> +\tEOF\n\nThis could be written with test_write_lines but a here-doc would be\na better option in this case, as we will see the expected output from\nthe tested command in the exact form.\n\n"},{"id":"406390","messageId":"xmqqv9g1k3os.fsf@gitster.c.googlers.com","threadId":"54298","inReplyTo":"CAPig+cTwQCSA1sjAWyDC5ZydT6NKzOY9ziLUJcOVMRV4cP_sAQ@mail.gmail.com","subject":"Re: [PATCH 07/11] t7001: use ': >' rather than 'touch'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T20:21:39Z","receivedAt":"2020-09-25T20:36:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> In patch [5/11] you dropped whitespace following the redirection\n> operator, however, this patch introduces several new cases of unwanted\n> whitespace.\n>\n> Checking \"master\", I see that there are 209 instances of `: >` in\n> tests, but 1023 instances of `>`, which suggests that we should stick\n> with plain `>` rather than `: >` in this patch.\n\nAgreed on both points.  Thanks for a careful review.\n\n"},{"id":"406391","messageId":"CAPig+cS+Hp3c96LjBezvhDg0WuBeFvPNVH9_V0iviEkFWfvjEQ@mail.gmail.com","threadId":"54298","inReplyTo":"20200925170256.11490-12-shubhunic@gmail.com","subject":"Re: [PATCH 11/11] t7001: move cleanup code from outside the tests into them","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-09-25T19:36:43Z","receivedAt":"2020-09-25T20:37:03Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> Let's use test_when_finished() to include cleanup code inside the tests,\n> as it's cleaner and safer to not have any code outside the tests.\n>\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> @@ -32,6 +32,7 @@ test_expect_success 'commiting the change' '\n>  test_expect_success 'checking the commit' '\n> +       test_when_finished \"rmdir path1\" &&\n>         git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n>         grep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n>  '\n\nThis one doesn't really pass the smell test. `path1` was created by\nthe \"prepare reference tree\" test; it is not created by this test, so\nit's not really this test's responsibility to clean it up. But it also\ncan't be cleaned up by \"prepare reference tree\" since that is just a\n\"setup\" test, and `path` is used by subsequent tests.\n\nDoes anything actually fail if directory `path1` is not removed? I ask\nbecause slightly below the point at which `path1` is removed (outside\nof any tests) a different test goes ahead and re-creates `path1`. If\nit turns out that removal of `path1` isn't actually necessary, then a\nbetter option might be simply to drop the global `rmdir path1`\naltogether, along with the subsequent `mkdir path1` which comes a bit\nlater.\n\n> @@ -43,6 +44,7 @@ test_expect_success 'mv --dry-run does not move file' '\n>  test_expect_success 'checking -k on non-existing file' '\n> +       test_when_finished \"rm -f idontexist path0/idontexist\" &&\n>         git mv -k idontexist path0\n>  '\n\nThe paths being removed here shouldn't actually be created by this\ntest, but if Git is somehow buggy and they do get created, then this\nis the appropriate place to clean them up. Good.\n\n> @@ -55,6 +57,7 @@ test_expect_success 'checking -k on untracked file' '\n>  test_expect_success 'checking -k on multiple untracked files' '\n>         : > untracked2 &&\n> +       test_when_finished \"rm -f untracked2 path0/untracked2\" &&\n>         git mv -k untracked1 untracked2 path0 &&\n>         test -f untracked1 &&\n>         test -f untracked2 &&\n> @@ -64,18 +67,14 @@ test_expect_success 'checking -k on multiple untracked files' '\n>  test_expect_success 'checking -f on untracked file with existing target' '\n>         : > path0/untracked1 &&\n> +       test_when_finished \"rm -f untracked1 path0/untracked1\" &&\n> +       test_when_finished \"rm -f .git/index.lock\" &&\n>         test_must_fail git mv -f untracked1 path0 &&\n>         test ! -f .git/index.lock &&\n>         test -f untracked1 &&\n>         test -f path0/untracked1\n>  '\n\nThis is a big ugly. `untracked1` gets created by an earlier test but\nis then cleaned up by this subsequent test. That goes against the idea\nof test_when_finished(), which is that tests should clean up after\nthemselves. Doing it this way also creates a smelly dependency between\nthe tests. What I would recommend instead is having each test\nindependently create and cleanup the \"untracked\" files it needs. This\nmakes the tests a tiny bit more verbose but makes it much clearer to\nthe reader that the tests are independent.\n\n> -# clean up the mess in case bad things happen\n> -rm -f idontexist untracked1 untracked2 \\\n> -     path0/idontexist path0/untracked1 path0/untracked2 \\\n> -     .git/index.lock\n> -rmdir path1\n> @@ -149,10 +148,12 @@ test_expect_success 'do not move directory over existing directory' '\n>  test_expect_success 'move into \".\"' '\n> +       test_when_finished \"rm -fr path?\" &&\n>         git mv path1/path2/ .\n>  '\n\nThis is another of those cases which doesn't really pass the smell\ntest. This may indeed be the final test in which the various `path?`\nsubdirectories are used, but it isn't the test which created them,\nthus it isn't \"cleaning up after itself\".\n\nIf the test which might get tripped up by these `path?` directories is\nthe \"Sergey Vlasov's test case\" test, then it probably would make more\nsense for _that_ test to do `rm -fr path?` as its very first step (not\nas a test_when_finished()) in order to prepare things the way it needs\nthem (just as it already does `rm -fr .git`).\n\n>  test_expect_success \"Michael Cassar's test case\" '\n> +       test_when_finished \"rm -fr papers partA\" &&\n>         rm -fr .git papers partA &&\n>         git init &&\n>         mkdir -p papers/unsorted papers/all-papers partA &&\n\nCleaning these paths here makes sense since they are created and only\nused by this test.\n\n> @@ -168,8 +169,6 @@ test_expect_success \"Michael Cassar's test case\" '\n> -rm -fr papers partA path?\n> -\n>  test_expect_success \"Sergey Vlasov's test case\" '\n>         rm -fr .git &&\n>         git init &&\n\nSo, given what I said above, the first line of this test might become:\n\n    rm -fr .git path? &&\n\n> @@ -230,6 +229,7 @@ test_expect_success 'git mv to move multiple sources into a directory' '\n>  test_expect_success 'git mv should not change sha1 of moved cache entry' '\n> +       test_when_finished \"rm -f dirty dirty2\" &&\n>         rm -fr .git &&\n>         git init &&\n>         echo 1 >dirty &&\n> @@ -242,8 +242,6 @@ test_expect_success 'git mv should not change sha1 of moved cache entry' '\n>         test \"$entry\" = \"$(git ls-files --stage dirty | cut -f 1)\"\n>  '\n>\n> -rm -f dirty dirty2\n\nMakes perfect sense.\n\n> @@ -262,6 +260,7 @@ test_expect_success 'git mv error on conflicted file' '\n>  test_expect_success 'git mv should overwrite symlink to a file' '\n> +       test_when_finished \"rm -f moved symlink\" &&\n>         rm -fr .git &&\n>         git init &&\n>         echo 1 >moved &&\n> @@ -276,9 +275,8 @@ test_expect_success 'git mv should overwrite symlink to a file' '\n>         git diff-files --quiet\n>  '\n>\n> -rm -f moved symlink\n\nOkay.\n\n>  test_expect_success 'git mv should overwrite file with a symlink' '\n> +       test_when_finished \"rm -f symlink\" &&\n>         rm -fr .git &&\n>         git init &&\n>         echo 1 >moved &&\n\nThis makes sense, but...\n\n> @@ -292,11 +290,10 @@ test_expect_success 'git mv should overwrite file with a symlink' '\n>  test_expect_success SYMLINKS 'check moved symlink' '\n> +       test_when_finished \"rm -f moved\" &&\n>         test -h moved\n>  '\n\n... this test only gets run on platforms which support symlinks (see\nthe SYMLINKS predicate in the test definition), so the `moved` file\nwon't get cleaned up on platforms which don't support symlinks.\n\n> -rm -f moved symlink\n\nIf the `moved` file actually causes subsequent tests to fail, then\nthis might be one of those rare instances in which you'd introduce a\ntest merely to clean up state from earlier tests. Perhaps something\nlike this:\n\n    test_expect_success 'cleanup symlink detritus' '\n        rm -r moved\n    '\n\nHowever, if `moved` doesn't cause subsequent tests to fail, then it\nmight also make sense instead just to leave it alone and not bother\ncleaning it up.\n"},{"id":"406393","messageId":"xmqqzh5dk3rq.fsf@gitster.c.googlers.com","threadId":"54298","inReplyTo":"CAPig+cQWvn0YXUZRHE2ocH8+FS1aAnhjKiMsQvXXkNQ7LgEmfg@mail.gmail.com","subject":"Re: [PATCH 03/11] t7001: remove unnecessary blank lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T20:19:53Z","receivedAt":"2020-09-25T20:38:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n> It is very uncommon style to hide the subshell as these two tests do:\n>\n>     test_expect_success 'title' '(\n>         ...\n>     )'\n>\n> Instead, these should be formatted as:\n>\n>     test_expect_success 'title' '\n>         (\n>             ...\n>         )\n>     '\n>\n> Note that the \"(\" and \")\" of the subshell are indented with a TAB, and\n> then the body of the subshell is indented again with another TAB in\n> order to comply with current style guidelines.\n>\n> Fixing these might possibly be done in patch [1/11], however, they are\n> so unusual and would change indentation of the body lines that they\n> might deserve a patch of their own to avoid being lost in the noise of\n> [1/11].\n\nI agree that adding that to 01/11 might be too noisy, but 04/11 may\nbe a good match.\n\nThanks.\n"},{"id":"406396","messageId":"xmqqlfgxk266.fsf@gitster.c.googlers.com","threadId":"54298","inReplyTo":"20200925170256.11490-12-shubhunic@gmail.com","subject":"Re: [PATCH 11/11] t7001: move cleanup code from outside the tests into them","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-09-25T20:54:25Z","receivedAt":"2020-09-25T20:54:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"shubham verma <shubhunic@gmail.com> writes:\n\n> From: Shubham Verma <shubhunic@gmail.com>\n>\n> Let's use test_when_finished() to include cleanup code inside the tests,\n> as it's cleaner and safer to not have any code outside the tests.\n>\n> Signed-off-by: shubham verma <shubhunic@gmail.com>\n> ---\n>  t/t7001-mv.sh | 25 +++++++++++--------------\n>  1 file changed, 11 insertions(+), 14 deletions(-)\n>\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index 7bb4a7b759..b4d04ceaf8 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -32,6 +32,7 @@ test_expect_success 'commiting the change' '\n>  '\n>  \n>  test_expect_success 'checking the commit' '\n> +\ttest_when_finished \"rmdir path1\" &&\n>  \tgit diff-tree -r -M --name-status  HEAD^ HEAD >actual &&\n>  \tgrep \"^R100..*path1/COPYING..*path0/COPYING\" actual\n>  '\n\nSorry, but why in this test?  It only runs diff-tree and runs grep,\nneither of which changes any state in the repository.  Because the\ntest does *not* create path1, and having or not having path1 on the\nfilesystem would not affect the outcome of the test, I do not see\nhow it makes sense to use test_when_finished in here.\n\nIf you are saying that path1 will no longer be used after this test\nfinishes, test_when_finished should be done in the test before this\none that used path1 the last, because this test does not care.  It\nis probably the one that moves the file out of path1 back to path0\nand records the result as a commit with title \"move-in\" (although if\nI were writing this test today, I would merge the \"move and commit\"\ninto one step).\n\n> @@ -43,6 +44,7 @@ test_expect_success 'mv --dry-run does not move file' '\n>  '\n>  \n>  test_expect_success 'checking -k on non-existing file' '\n> +\ttest_when_finished \"rm -f idontexist path0/idontexist\" &&\n>  \tgit mv -k idontexist path0\n>  '\n\nI do not see the point of \"rm -f idontexist\" in the\npost-test-cleanup at all.  Some might see that path0/ideontexist is\nworth having there, in case the \"mv\" command gets so broken that it\ncreates such a file by mistake, but Personally I'd prefer to use\ntest_when_finished to clean up the side effects we _expect_ to\ncause.  We cannot anticipate each and every breakage.\n\nThe other side of the coin is that this test DEPENDS ON the fact\nthat idontexist does *NOT* exist before it runs \"git mv -k\nidontexist path0\", but nobody before us gives us any explicitly\nguarantee.  This test also depends on the presence of path0\ndirectory the same way.  Instead of relying on others that came\nbefore us to have cleaned after themselves for us, we can more\nexplicitly protect ourselves by making sure the pre-condition we\ndepend on holds.  I.e.\n\n    test_expect_success 'mv -k on non-exising file would not fail' '\n\tmkdir -p path0 &&\n\trm -f idontexist path0/idontexist &&\n\tgit mv -k idontexist path0\n    '\n\nA broken \"git mv\" may or may not leave path0/idontexist behind, but\nas long as the tests that come after us protect themselves with the\nsame principle of making sure the preconditions they care about do\nhold, we do not necessarily have to clean after ourselves.  Since we\nexpect we do not leave any side effect, I'd rather not to use\ntest_when_finished here.\n\n@@ -55,6 +57,7 @@ test_expect_success 'checking -k on untracked file' '\n>  \n>  test_expect_success 'checking -k on multiple untracked files' '\n>  \t: > untracked2 &&\n> +\ttest_when_finished \"rm -f untracked2 path0/untracked2\" &&\n>  \tgit mv -k untracked1 untracked2 path0 &&\n>  \ttest -f untracked1 &&\n>  \ttest -f untracked2 &&\n\nAn exercise to readers.  Explain why\n\n - we want to move test_when_finished _before_ \">untracked2\" is created;\n\n - \"rm -f untrackd2\" in test_when_finished is a good idea;\n\n - \"rm -f path0/untracked2\" is not a good idea;\n\n - we may want to do\n\n\t>untracked1 &&\n\tmkdir -p path0 &&\n\n   before \"git mv -k ...\" is tested.\n\n> -# clean up the mess in case bad things happen\n> -rm -f idontexist untracked1 untracked2 \\\n> -     path0/idontexist path0/untracked1 path0/untracked2 \\\n> -     .git/index.lock\n> -rmdir path1\n> -\n>  test_expect_success 'moving to absent target with trailing slash' '\n>  \ttest_must_fail git mv path0/COPYING no-such-dir/ &&\n>  \ttest_must_fail git mv path0/COPYING no-such-dir// &&\n\nIt may be a better approach to move the above removals at the\nbeginning of this test, just before the first test_must_fail line.\n\n> -rm -fr papers partA path?\n> -\n>  test_expect_success \"Sergey Vlasov's test case\" '\n\nLikewise.\n"},{"id":"406753","messageId":"CAD=kLpvWo3AuBsY8-tQSiuTOnAt5hp-hyO90SzCX5fFmKfahuQ@mail.gmail.com","threadId":"54298","inReplyTo":"CAPig+cTwHuTA=puzPfpOiMcMUkYPsbOca1GQK=dS1hJJeZZBGg@mail.gmail.com","subject":"Re: [PATCH 00/11] Modernizing the t7001 test script","fromName":"Shubham Verma","fromEmail":"shubhunic@gmail.com","sentAt":"2020-10-01T05:42:23Z","receivedAt":"2020-10-01T05:42:37Z","isPatch":true,"sender":{"key":"shubhunic@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45103446?v=4"},"body":"On Fri, Sep 25, 2020 at 11:03 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>\n> On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n> > In this patch series modernize the t7001 test script by changing the\n> > style of its tests from an old one to the modern one and by cleaning\n> > up the test script.\n>\n> Thanks for tackling this task. I presume it was prompted by [1] or\n> [2], as this series covers many of the items mentioned in [1].\n> Overall, the series looks good. I'll leave comments in a few of the\n> individual patches.\n>\n> [1]: https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/\n> [2]: https://git.github.io/rev_news/2020/08/27/edition-66/\n\nEric, Actually I follow the Instruction that you pointed out in [1].\nOkay, I improve the commits and make changes according to your comments.\n\nThank You!\n"},{"id":"412851","messageId":"xmqqk0t9ir2n.fsf@gitster.c.googlers.com","threadId":"54298","inReplyTo":"CAD=kLpvWo3AuBsY8-tQSiuTOnAt5hp-hyO90SzCX5fFmKfahuQ@mail.gmail.com","subject":"Re: [PATCH 00/11] Modernizing the t7001 test script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-12-22T19:22:24Z","receivedAt":"2020-12-22T19:23:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shubham Verma <shubhunic@gmail.com> writes:\n\n> On Fri, Sep 25, 2020 at 11:03 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n>>\n>> On Fri, Sep 25, 2020 at 1:03 PM shubham verma <shubhunic@gmail.com> wrote:\n>> > In this patch series modernize the t7001 test script by changing the\n>> > style of its tests from an old one to the modern one and by cleaning\n>> > up the test script.\n>>\n>> Thanks for tackling this task. I presume it was prompted by [1] or\n>> [2], as this series covers many of the items mentioned in [1].\n>> Overall, the series looks good. I'll leave comments in a few of the\n>> individual patches.\n>>\n>> [1]: https://lore.kernel.org/git/CAPig+cQpUu2UO-+jWn1nTaDykWnxwuEitzVB7PnW2SS_b7V8Hg@mail.gmail.com/\n>> [2]: https://git.github.io/rev_news/2020/08/27/edition-66/\n>\n> Eric, Actually I follow the Instruction that you pointed out in [1].\n> Okay, I improve the commits and make changes according to your comments.\n>\n> Thank You!\n\nThis series has seen quite a many review comments, and as far as I\nremember, none of them was a suggestion that was hard to decipher.\n\nIt has been almost 3 months---has an update been posted that I\nmissed?\n\nThanks.\n"}]}