On 9/21/2018 7:58 PM, Stefan Beller wrote:
The old formatting style is a real hindrance of getting people up to speed
contributing as they use existing code as an example and follow that style.
So let's get rid of the old style and reformat it in our current style.
I was suspicious of your automated approach catching everything, so I looked carefully at this patch. There are still a lot of things happening that we would not recommend doing in new tests.

Reported-by: Derrick Stolee <sto...@gmail.com>
Reported-by: Jeff Hostetler <g...@jeffhostetler.com>
Signed-off-by: Stefan Beller <sbel...@google.com>
---
  t/t7001-mv.sh | 268 +++++++++++++++++++++++++-------------------------
  1 file changed, 134 insertions(+), 134 deletions(-)

diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index 36b50d0b4c1..2251d24735c 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -3,74 +3,74 @@
  test_description='git mv in subdirs'
  . ./test-lib.sh
-test_expect_success \
-    'prepare reference tree' \
-    'mkdir path0 path1 &&
-     cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
-     git add path0/COPYING &&
-     git commit -m add -a'
+test_expect_success 'prepare reference tree' '
+       mkdir path0 path1 &&
+       cp "$TEST_DIRECTORY"/../COPYING path0/COPYING &&
+       git add path0/COPYING &&
+       git commit -m add -a
+'
-test_expect_success \
-    'moving the file out of subdirectory' \
-    'cd path0 && git mv COPYING ../path1/COPYING'
+test_expect_success 'moving the file out of subdirectory' '
+       cd path0 && git mv COPYING ../path1/COPYING
+'
Perhaps split this line on the &&?
# in path0 currently
-test_expect_success \
-    'commiting the change' \
-    'cd .. && git commit -m move-out -a'
+test_expect_success 'commiting the change' '
+       cd .. && git commit -m move-out -a
+'

This "cd .." should probably be removed and use a subshell in the test above where we "cd path0".

-test_expect_success \
-    'checking the commit' \
-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
-    grep "^R100..*path0/COPYING..*path1/COPYING" actual'
+test_expect_success 'checking the commit' '
+       git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
+       grep "^R100..*path0/COPYING..*path1/COPYING" actual
+'
-test_expect_success \
-    'moving the file back into subdirectory' \
-    'cd path0 && git mv ../path1/COPYING COPYING'
+test_expect_success 'moving the file back into subdirectory' '
+       cd path0 && git mv ../path1/COPYING COPYING
+'

Split at &&, use subshell?


+test_expect_success 'commiting the change' '
+       cd .. && git commit -m move-in -a
+'

Drop "cd .." (and the comments about being in path0)

[big snip]

+test_expect_success 'moving to existing tracked target with trailing slash' '
+       mkdir path2 &&
+       >path2/file && git add path2/file &&
This line in particular looks a bit strange. What is this doing? At least we should split the &&.
+       git mv path1/path0/ path2/ &&
+       test_path_is_dir path2/path0/
+'
+
+test_expect_success 'clean up' '
+       git reset --hard
+'
+
+test_expect_success 'adding another file' '
+       cp "$TEST_DIRECTORY"/../README.md path0/README &&
+       git add path0/README &&
+       git commit -m add2 -a
+'
+
+test_expect_success 'moving whole subdirectory' '
+       git mv path0 path2
+'
+
+test_expect_success 'commiting the change' '
+       git commit -m dir-move -a
+'
+
+test_expect_success 'checking the commit' '
+       git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
+       grep "^R100..*path0/COPYING..*path2/COPYING" actual &&
+       grep "^R100..*path0/README..*path2/README" actual
+'
+
+test_expect_success 'succeed when source is a prefix of destination' '
+       git mv path2/COPYING path2/COPYING-renamed
+'
+
+test_expect_success 'moving whole subdirectory into subdirectory' '
+       git mv path2 path1
+'
+
+test_expect_success 'commiting the change' '
+       git commit -m dir-move -a
+'
+
+test_expect_success 'checking the commit' '
+       git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
+       grep "^R100..*path2/COPYING..*path1/path2/COPYING" actual &&
+       grep "^R100..*path2/README..*path1/path2/README" actual
+'
+
+test_expect_success 'do not move directory over existing directory' '
+       mkdir path0 && mkdir path0/path2 && test_must_fail git mv path2 path0
+'

Split this line.

Thanks,

-Stolee

Reply via email to