git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/3] bring some tests to newer style.
@ 2018-09-21 23:58 Stefan Beller
  2018-09-21 23:58 ` [PATCH 1/3] t7001: reformat " Stefan Beller
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Stefan Beller @ 2018-09-21 23:58 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

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.

This was reported off list by Derrick and Jeff as both of them followed
outdated formatting style for their first patches only to have a long
discussion on the mailing list on style.

The tests changed are not modified in origin/master..origin/pu,
and they were changed using a hacky script:
--8<--
#!/usr/bin/python
import sys
def applyformat(fname):
	with open(fname, 'r') as f:
		lines = f.readlines()
		state = "lookingforstart"
		outlines = []
		for line in lines:
			if state == "lookingforstart":
				if line == "test_expect_success \\\n":
					print "AHA!"
					state = "firstlinefound"
				else:
					outlines += [line]
			elif state == "firstlinefound":
				l = line.strip()
				must_strip = False
				if l.endswith("\\"):
					l = l[:-1]
					must_strip=True
				if l.endswith("'"):
					l = l[:-1].strip()
				if l.startswith("'"):
					line = "test_expect_success " + l + " '\n"
					outlines += [line]
					state = "re-indent-until-done"
				else:
					print "what?"
					exit(1)
			elif state == "re-indent-until-done":
				l = line.strip()
				if must_strip:
					if l.startswith("'"):
						l = l[1:]
						must_strip = False
					else:
						print "what 1?"
						exit(1)
				if l.endswith("'"):
					l = l[:-1]
					state = "lookingforstart"

				if len(l):
					line = "	" + l + "\n"
					outlines += [line]
				elif state == "lookingforstart":
					# skip an empty line before test is done
					pass
				else:
					outlines += ["\n"]

				if state == "lookingforstart":
					outlines += ["'\n"]
			else:
				print "what?"
				exit(1)
	with open(fname, 'w') as f:
		f.write(''.join(outlines))

for n in sys.argv[1:]:
	print n
	applyformat(n)
--8<--

Thanks,
Stefan

Stefan Beller (3):
  t7001: reformat to newer style
  t7004: reformat style
  t0030: reformat style

 t/t0030-stripspace.sh | 525 ++++++++++++++++++++----------------------
 t/t7001-mv.sh         | 268 ++++++++++-----------
 t/t7004-tag.sh        | 149 +++++-------
 3 files changed, 444 insertions(+), 498 deletions(-)

-- 
2.19.0.444.g18242da7ef-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] t7001: reformat to newer style
  2018-09-21 23:58 [PATCH 0/3] bring some tests to newer style Stefan Beller
@ 2018-09-21 23:58 ` Stefan Beller
  2018-09-24 13:31   ` Derrick Stolee
  2018-09-21 23:58 ` [PATCH 2/3] t7004: reformat style Stefan Beller
  2018-09-21 23:58 ` [PATCH 3/3] t0030: " Stefan Beller
  2 siblings, 1 reply; 8+ messages in thread
From: Stefan Beller @ 2018-09-21 23:58 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

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.

Reported-by: Derrick Stolee <stolee@gmail.com>
Reported-by: Jeff Hostetler <git@jeffhostetler.com>
Signed-off-by: Stefan Beller <sbeller@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
+'
 
 # 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
+'
 
-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
+'
 
 # in path0 currently
-test_expect_success \
-    'commiting the change' \
-    'cd .. && git commit -m move-in -a'
-
-test_expect_success \
-    'checking the commit' \
-    'git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
-    grep "^R100..*path1/COPYING..*path0/COPYING" actual'
-
-test_expect_success \
-    'mv --dry-run does not move file' \
-    'git mv -n path0/COPYING MOVED &&
-     test -f path0/COPYING &&
-     test ! -f MOVED'
-
-test_expect_success \
-    'checking -k on non-existing file' \
-    'git mv -k idontexist path0'
-
-test_expect_success \
-    'checking -k on untracked file' \
-    'touch untracked1 &&
-     git mv -k untracked1 path0 &&
-     test -f untracked1 &&
-     test ! -f path0/untracked1'
-
-test_expect_success \
-    'checking -k on multiple untracked files' \
-    'touch untracked2 &&
-     git mv -k untracked1 untracked2 path0 &&
-     test -f untracked1 &&
-     test -f untracked2 &&
-     test ! -f path0/untracked1 &&
-     test ! -f path0/untracked2'
-
-test_expect_success \
-    'checking -f on untracked file with existing target' \
-    'touch path0/untracked1 &&
-     test_must_fail git mv -f untracked1 path0 &&
-     test ! -f .git/index.lock &&
-     test -f untracked1 &&
-     test -f path0/untracked1'
+test_expect_success 'commiting the change' '
+	cd .. && git commit -m move-in -a
+'
+
+test_expect_success 'checking the commit' '
+	git diff-tree -r -M --name-status  HEAD^ HEAD >actual &&
+	grep "^R100..*path1/COPYING..*path0/COPYING" actual
+'
+
+test_expect_success 'mv --dry-run does not move file' '
+	git mv -n path0/COPYING MOVED &&
+	test -f path0/COPYING &&
+	test ! -f MOVED
+'
+
+test_expect_success 'checking -k on non-existing file' '
+	git mv -k idontexist path0
+'
+
+test_expect_success 'checking -k on untracked file' '
+	touch untracked1 &&
+	git mv -k untracked1 path0 &&
+	test -f untracked1 &&
+	test ! -f path0/untracked1
+'
+
+test_expect_success 'checking -k on multiple untracked files' '
+	touch untracked2 &&
+	git mv -k untracked1 untracked2 path0 &&
+	test -f untracked1 &&
+	test -f untracked2 &&
+	test ! -f path0/untracked1 &&
+	test ! -f path0/untracked2
+'
+
+test_expect_success 'checking -f on untracked file with existing target' '
+	touch path0/untracked1 &&
+	test_must_fail git mv -f untracked1 path0 &&
+	test ! -f .git/index.lock &&
+	test -f untracked1 &&
+	test -f path0/untracked1
+'
 
 # clean up the mess in case bad things happen
 rm -f idontexist untracked1 untracked2 \
@@ -78,79 +78,79 @@ rm -f idontexist untracked1 untracked2 \
      .git/index.lock
 rmdir path1
 
-test_expect_success \
-    'moving to absent target with trailing slash' \
-    'test_must_fail git mv path0/COPYING no-such-dir/ &&
-     test_must_fail git mv path0/COPYING no-such-dir// &&
-     git mv path0/ no-such-dir/ &&
-     test_path_is_dir no-such-dir'
-
-test_expect_success \
-    'clean up' \
-    'git reset --hard'
-
-test_expect_success \
-    'moving to existing untracked target with trailing slash' \
-    'mkdir path1 &&
-     git mv path0/ path1/ &&
-     test_path_is_dir path1/path0/'
-
-test_expect_success \
-    'moving to existing tracked target with trailing slash' \
-    'mkdir path2 &&
-     >path2/file && git add path2/file &&
-     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'
-
-test_expect_success \
-    'move into "."' \
-    'git mv path1/path2/ .'
+test_expect_success 'moving to absent target with trailing slash' '
+	test_must_fail git mv path0/COPYING no-such-dir/ &&
+	test_must_fail git mv path0/COPYING no-such-dir// &&
+	git mv path0/ no-such-dir/ &&
+	test_path_is_dir no-such-dir
+'
+
+test_expect_success 'clean up' '
+	git reset --hard
+'
+
+test_expect_success 'moving to existing untracked target with trailing slash' '
+	mkdir path1 &&
+	git mv path0/ path1/ &&
+	test_path_is_dir path1/path0/
+'
+
+test_expect_success 'moving to existing tracked target with trailing slash' '
+	mkdir path2 &&
+	>path2/file && git add path2/file &&
+	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
+'
+
+test_expect_success 'move into "."' '
+	git mv path1/path2/ .
+'
 
 test_expect_success "Michael Cassar's test case" '
 	rm -fr .git papers partA &&
-- 
2.19.0.444.g18242da7ef-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] t7004: reformat style
  2018-09-21 23:58 [PATCH 0/3] bring some tests to newer style Stefan Beller
  2018-09-21 23:58 ` [PATCH 1/3] t7001: reformat " Stefan Beller
@ 2018-09-21 23:58 ` Stefan Beller
  2018-09-21 23:58 ` [PATCH 3/3] t0030: " Stefan Beller
  2 siblings, 0 replies; 8+ messages in thread
From: Stefan Beller @ 2018-09-21 23:58 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t7004-tag.sh | 149 +++++++++++++++++++------------------------------
 1 file changed, 56 insertions(+), 93 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 0b01862c23a..03a96b7f79e 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -144,26 +144,25 @@ test_expect_success 'listing a tag using a matching pattern should succeed' \
 test_expect_success 'listing a tag with --ignore-case' \
 	'test $(git tag -l --ignore-case MYTAG) = mytag'
 
-test_expect_success \
-	'listing a tag using a matching pattern should output that tag' \
-	'test $(git tag -l mytag) = mytag'
+test_expect_success 'listing a tag using a matching pattern should output that tag'  '
+	test $(git tag -l mytag) = mytag
+'
 
-test_expect_success \
-	'listing tags using a non-matching pattern should succeed' \
-	'git tag -l xxx'
+test_expect_success 'listing tags using a non-matching pattern should succeed'  '
+	git tag -l xxx
+'
 
-test_expect_success \
-	'listing tags using a non-matching pattern should output nothing' \
-	'test $(git tag -l xxx | wc -l) -eq 0'
+test_expect_success 'listing tags using a non-matching pattern should output nothing'  '
+	test $(git tag -l xxx | wc -l) -eq 0
+'
 
 # special cases for creating tags:
 
-test_expect_success \
-	'trying to create a tag with the name of one existing should fail' \
-	'test_must_fail git tag mytag'
+test_expect_success 'trying to create a tag with the name of one existing should fail'  '
+	test_must_fail git tag mytag
+'
 
-test_expect_success \
-	'trying to create a tag with a non-valid name should fail' '
+test_expect_success 'trying to create a tag with a non-valid name should fail' '
 	test $(git tag -l | wc -l) -eq 1 &&
 	test_must_fail git tag "" &&
 	test_must_fail git tag .othertag &&
@@ -201,15 +200,13 @@ cat >expect <<EOF
 myhead
 mytag
 EOF
-test_expect_success \
-	'trying to delete tags without params should succeed and do nothing' '
+test_expect_success 'trying to delete tags without params should succeed and do nothing' '
 	git tag -l > actual && test_cmp expect actual &&
 	git tag -d &&
 	git tag -l > actual && test_cmp expect actual
 '
 
-test_expect_success \
-	'deleting two existing tags in one command should succeed' '
+test_expect_success 'deleting two existing tags in one command should succeed' '
 	tag_exists mytag &&
 	tag_exists myhead &&
 	git tag -d mytag myhead &&
@@ -217,15 +214,13 @@ test_expect_success \
 	! tag_exists myhead
 '
 
-test_expect_success \
-	'creating a tag with the name of another deleted one should succeed' '
+test_expect_success 'creating a tag with the name of another deleted one should succeed' '
 	! tag_exists mytag &&
 	git tag mytag &&
 	tag_exists mytag
 '
 
-test_expect_success \
-	'trying to delete two tags, existing and not, should fail in the 2nd' '
+test_expect_success 'trying to delete two tags, existing and not, should fail in the 2nd' '
 	tag_exists mytag &&
 	! tag_exists myhead &&
 	test_must_fail git tag -d mytag anothertag &&
@@ -270,8 +265,7 @@ a1
 aa1
 cba
 EOF
-test_expect_success \
-	'listing tags with substring as pattern must print those matching' '
+test_expect_success 'listing tags with substring as pattern must print those matching' '
 	rm *a* &&
 	git tag -l "*a*" > current &&
 	test_cmp expect current
@@ -281,8 +275,7 @@ cat >expect <<EOF
 v0.2.1
 v1.0.1
 EOF
-test_expect_success \
-	'listing tags with a suffix as pattern must print those matching' '
+test_expect_success 'listing tags with a suffix as pattern must print those matching' '
 	git tag -l "*.1" > actual &&
 	test_cmp expect actual
 '
@@ -291,8 +284,7 @@ cat >expect <<EOF
 t210
 t211
 EOF
-test_expect_success \
-	'listing tags with a prefix as pattern must print those matching' '
+test_expect_success 'listing tags with a prefix as pattern must print those matching' '
 	git tag -l "t21*" > actual &&
 	test_cmp expect actual
 '
@@ -300,8 +292,7 @@ test_expect_success \
 cat >expect <<EOF
 a1
 EOF
-test_expect_success \
-	'listing tags using a name as pattern must print that one matching' '
+test_expect_success 'listing tags using a name as pattern must print that one matching' '
 	git tag -l a1 > actual &&
 	test_cmp expect actual
 '
@@ -309,8 +300,7 @@ test_expect_success \
 cat >expect <<EOF
 v1.0
 EOF
-test_expect_success \
-	'listing tags using a name as pattern must print that one matching' '
+test_expect_success 'listing tags using a name as pattern must print that one matching' '
 	git tag -l v1.0 > actual &&
 	test_cmp expect actual
 '
@@ -319,14 +309,12 @@ cat >expect <<EOF
 v1.0.1
 v1.1.3
 EOF
-test_expect_success \
-	'listing tags with ? in the pattern should print those matching' '
+test_expect_success 'listing tags with ? in the pattern should print those matching' '
 	git tag -l "v1.?.?" > actual &&
 	test_cmp expect actual
 '
 
-test_expect_success \
-	'listing tags using v.* should print nothing because none have v.' '
+test_expect_success 'listing tags using v.* should print nothing because none have v.' '
 	git tag -l "v.*" > actual &&
 	test_must_be_empty actual
 '
@@ -337,8 +325,7 @@ v1.0
 v1.0.1
 v1.1.3
 EOF
-test_expect_success \
-	'listing tags using v* should print only those having v' '
+test_expect_success 'listing tags using v* should print only those having v' '
 	git tag -l "v*" > actual &&
 	test_cmp expect actual
 '
@@ -404,8 +391,7 @@ EOF
 
 # creating and verifying lightweight tags:
 
-test_expect_success \
-	'a non-annotated tag created without parameters should point to HEAD' '
+test_expect_success 'a non-annotated tag created without parameters should point to HEAD' '
 	git tag non-annotated-tag &&
 	test $(git cat-file -t non-annotated-tag) = commit &&
 	test $(git rev-parse non-annotated-tag) = $(git rev-parse HEAD)
@@ -414,13 +400,13 @@ test_expect_success \
 test_expect_success 'trying to verify an unknown tag should fail' \
 	'test_must_fail git tag -v unknown-tag'
 
-test_expect_success \
-	'trying to verify a non-annotated and non-signed tag should fail' \
-	'test_must_fail git tag -v non-annotated-tag'
+test_expect_success 'trying to verify a non-annotated and non-signed tag should fail'  '
+	test_must_fail git tag -v non-annotated-tag
+'
 
-test_expect_success \
-	'trying to verify many non-annotated or unknown tags, should fail' \
-	'test_must_fail git tag -v unknown-tag1 non-annotated-tag unknown-tag2'
+test_expect_success 'trying to verify many non-annotated or unknown tags, should fail'  '
+	test_must_fail git tag -v unknown-tag1 non-annotated-tag unknown-tag2
+'
 
 # creating annotated tags:
 
@@ -444,8 +430,7 @@ time=$test_tick
 
 get_tag_header annotated-tag $commit commit $time >expect
 echo "A message" >>expect
-test_expect_success \
-	'creating an annotated tag with -m message should succeed' '
+test_expect_success 'creating an annotated tag with -m message should succeed' '
 	git tag -m "A message" annotated-tag &&
 	get_tag_msg annotated-tag >actual &&
 	test_cmp expect actual
@@ -459,8 +444,7 @@ test_expect_success 'set up editor' '
 	mv "$1-" "$1"
 	EOF
 '
-test_expect_success \
-	'creating an annotated tag with -m message --edit should succeed' '
+test_expect_success 'creating an annotated tag with -m message --edit should succeed' '
 	GIT_EDITOR=./fakeeditor git tag -m "A message" --edit annotated-tag-edit &&
 	get_tag_msg annotated-tag-edit >actual &&
 	test_cmp expect actual
@@ -472,8 +456,7 @@ in a file.
 EOF
 get_tag_header file-annotated-tag $commit commit $time >expect
 cat msgfile >>expect
-test_expect_success \
-	'creating an annotated tag with -F messagefile should succeed' '
+test_expect_success 'creating an annotated tag with -F messagefile should succeed' '
 	git tag -F msgfile file-annotated-tag &&
 	get_tag_msg file-annotated-tag >actual &&
 	test_cmp expect actual
@@ -487,8 +470,7 @@ test_expect_success 'set up editor' '
 	mv "$1-" "$1"
 	EOF
 '
-test_expect_success \
-	'creating an annotated tag with -F messagefile --edit should succeed' '
+test_expect_success 'creating an annotated tag with -F messagefile --edit should succeed' '
 	GIT_EDITOR=./fakeeditor git tag -F msgfile --edit file-annotated-tag-edit &&
 	get_tag_msg file-annotated-tag-edit >actual &&
 	test_cmp expect actual
@@ -506,16 +488,14 @@ test_expect_success 'creating an annotated tag with -F - should succeed' '
 	test_cmp expect actual
 '
 
-test_expect_success \
-	'trying to create a tag with a non-existing -F file should fail' '
+test_expect_success 'trying to create a tag with a non-existing -F file should fail' '
 	! test -f nonexistingfile &&
 	! tag_exists notag &&
 	test_must_fail git tag -F nonexistingfile notag &&
 	! tag_exists notag
 '
 
-test_expect_success \
-	'trying to create tags giving both -m or -F options should fail' '
+test_expect_success 'trying to create tags giving both -m or -F options should fail' '
 	echo "message file 1" >msgfile1 &&
 	echo "message file 2" >msgfile2 &&
 	! tag_exists msgtag &&
@@ -524,15 +504,14 @@ test_expect_success \
 	test_must_fail git tag -F msgfile1 -m "message 1" msgtag &&
 	! tag_exists msgtag &&
 	test_must_fail git tag -m "message 1" -F msgfile1 \
-		-m "message 2" msgtag &&
+	-m "message 2" msgtag &&
 	! tag_exists msgtag
 '
 
 # blank and empty messages:
 
 get_tag_header empty-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with an empty -m message should succeed' '
+test_expect_success 'creating a tag with an empty -m message should succeed' '
 	git tag -m "" empty-annotated-tag &&
 	get_tag_msg empty-annotated-tag >actual &&
 	test_cmp expect actual
@@ -540,8 +519,7 @@ test_expect_success \
 
 >emptyfile
 get_tag_header emptyfile-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with an empty -F messagefile should succeed' '
+test_expect_success 'creating a tag with an empty -F messagefile should succeed' '
 	git tag -F emptyfile emptyfile-annotated-tag &&
 	get_tag_msg emptyfile-annotated-tag >actual &&
 	test_cmp expect actual
@@ -561,16 +539,14 @@ Trailing spaces
 
 Trailing blank lines
 EOF
-test_expect_success \
-	'extra blanks in the message for an annotated tag should be removed' '
+test_expect_success 'extra blanks in the message for an annotated tag should be removed' '
 	git tag -F blanksfile blanks-annotated-tag &&
 	get_tag_msg blanks-annotated-tag >actual &&
 	test_cmp expect actual
 '
 
 get_tag_header blank-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with blank -m message with spaces should succeed' '
+test_expect_success 'creating a tag with blank -m message with spaces should succeed' '
 	git tag -m "     " blank-annotated-tag &&
 	get_tag_msg blank-annotated-tag >actual &&
 	test_cmp expect actual
@@ -580,8 +556,7 @@ echo '     ' >blankfile
 echo ''      >>blankfile
 echo '  '    >>blankfile
 get_tag_header blankfile-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with blank -F messagefile with spaces should succeed' '
+test_expect_success 'creating a tag with blank -F messagefile with spaces should succeed' '
 	git tag -F blankfile blankfile-annotated-tag &&
 	get_tag_msg blankfile-annotated-tag >actual &&
 	test_cmp expect actual
@@ -589,8 +564,7 @@ test_expect_success \
 
 printf '      ' >blanknonlfile
 get_tag_header blanknonlfile-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with -F file of spaces and no newline should succeed' '
+test_expect_success 'creating a tag with -F file of spaces and no newline should succeed' '
 	git tag -F blanknonlfile blanknonlfile-annotated-tag &&
 	get_tag_msg blanknonlfile-annotated-tag >actual &&
 	test_cmp expect actual
@@ -624,16 +598,14 @@ Another line.
 
 Last line.
 EOF
-test_expect_success \
-	'creating a tag using a -F messagefile with #comments should succeed' '
+test_expect_success 'creating a tag using a -F messagefile with #comments should succeed' '
 	git tag -F commentsfile comments-annotated-tag &&
 	get_tag_msg comments-annotated-tag >actual &&
 	test_cmp expect actual
 '
 
 get_tag_header comment-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with a #comment in the -m message should succeed' '
+test_expect_success 'creating a tag with a #comment in the -m message should succeed' '
 	git tag -m "#comment" comment-annotated-tag &&
 	get_tag_msg comment-annotated-tag >actual &&
 	test_cmp expect actual
@@ -643,8 +615,7 @@ echo '#comment' >commentfile
 echo ''         >>commentfile
 echo '####'     >>commentfile
 get_tag_header commentfile-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with #comments in the -F messagefile should succeed' '
+test_expect_success 'creating a tag with #comments in the -F messagefile should succeed' '
 	git tag -F commentfile commentfile-annotated-tag &&
 	get_tag_msg commentfile-annotated-tag >actual &&
 	test_cmp expect actual
@@ -652,8 +623,7 @@ test_expect_success \
 
 printf '#comment' >commentnonlfile
 get_tag_header commentnonlfile-annotated-tag $commit commit $time >expect
-test_expect_success \
-	'creating a tag with a file of #comment and no newline should succeed' '
+test_expect_success 'creating a tag with a file of #comment and no newline should succeed' '
 	git tag -F commentnonlfile commentnonlfile-annotated-tag &&
 	get_tag_msg commentnonlfile-annotated-tag >actual &&
 	test_cmp expect actual
@@ -661,8 +631,7 @@ test_expect_success \
 
 # listing messages for annotated non-signed tags:
 
-test_expect_success \
-	'listing the one-line message of a non-signed tag should succeed' '
+test_expect_success 'listing the one-line message of a non-signed tag should succeed' '
 	git tag -m "A msg" tag-one-line &&
 
 	echo "tag-one-line" >expect &&
@@ -701,8 +670,7 @@ test_expect_success 'The -n 100 invocation means -n --list 100, not -n100' '
 	test_cmp expect actual
 '
 
-test_expect_success \
-	'listing the zero-lines message of a non-signed tag should succeed' '
+test_expect_success 'listing the zero-lines message of a non-signed tag should succeed' '
 	git tag -m "" tag-zero-lines &&
 
 	echo "tag-zero-lines" >expect &&
@@ -729,8 +697,7 @@ test_expect_success \
 echo 'tag line one' >annotagmsg
 echo 'tag line two' >>annotagmsg
 echo 'tag line three' >>annotagmsg
-test_expect_success \
-	'listing many message lines of a non-signed tag should succeed' '
+test_expect_success 'listing many message lines of a non-signed tag should succeed' '
 	git tag -F annotagmsg tag-lines &&
 
 	echo "tag-lines" >expect &&
@@ -1372,13 +1339,11 @@ test_expect_success GPG \
 	'verify signed tag fails when public key is not present' \
 	'test_must_fail git tag -v signed-tag'
 
-test_expect_success \
-	'git tag -a fails if tag annotation is empty' '
+test_expect_success 'git tag -a fails if tag annotation is empty' '
 	! (GIT_EDITOR=cat git tag -a initial-comment)
 '
 
-test_expect_success \
-	'message in editor has initial comment' '
+test_expect_success 'message in editor has initial comment' '
 	! (GIT_EDITOR=cat git tag -a initial-comment > actual)
 '
 
@@ -1389,8 +1354,7 @@ test_expect_success 'message in editor has initial comment: first line' '
 	test_i18ncmp first.expect first.actual
 '
 
-test_expect_success \
-	'message in editor has initial comment: remainder' '
+test_expect_success 'message in editor has initial comment: remainder' '
 	# remove commented lines from the remainder -- should be empty
 	sed -e 1d -e "/^#/d" <actual >rest.actual &&
 	test_must_be_empty rest.actual
@@ -1398,8 +1362,7 @@ test_expect_success \
 
 get_tag_header reuse $commit commit $time >expect
 echo "An annotation to be reused" >> expect
-test_expect_success \
-	'overwriting an annoted tag should use its previous body' '
+test_expect_success 'overwriting an annoted tag should use its previous body' '
 	git tag -a -m "An annotation to be reused" reuse &&
 	GIT_EDITOR=true git tag -f -a reuse &&
 	get_tag_msg reuse >actual &&
-- 
2.19.0.444.g18242da7ef-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] t0030: reformat style
  2018-09-21 23:58 [PATCH 0/3] bring some tests to newer style Stefan Beller
  2018-09-21 23:58 ` [PATCH 1/3] t7001: reformat " Stefan Beller
  2018-09-21 23:58 ` [PATCH 2/3] t7004: reformat style Stefan Beller
@ 2018-09-21 23:58 ` Stefan Beller
  2 siblings, 0 replies; 8+ messages in thread
From: Stefan Beller @ 2018-09-21 23:58 UTC (permalink / raw)
  To: git; +Cc: Stefan Beller

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/t0030-stripspace.sh | 525 ++++++++++++++++++++----------------------
 1 file changed, 254 insertions(+), 271 deletions(-)

diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
index 5ce47e8af51..c2281a39b58 100755
--- a/t/t0030-stripspace.sh
+++ b/t/t0030-stripspace.sh
@@ -12,379 +12,362 @@ s40='                                        '
 sss="$s40$s40$s40$s40$s40$s40$s40$s40$s40$s40" # 400
 ttt="$t40$t40$t40$t40$t40$t40$t40$t40$t40$t40" # 400
 
-test_expect_success \
-    'long lines without spaces should be unchanged' '
-    echo "$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
-
-    echo "$ttt$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
-
-    echo "$ttt$ttt$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
-
-    echo "$ttt$ttt$ttt$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual
+test_expect_success 'long lines without spaces should be unchanged' '
+	echo "$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
+
+	echo "$ttt$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
+
+	echo "$ttt$ttt$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
+
+	echo "$ttt$ttt$ttt$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'lines with spaces at the beginning should be unchanged' '
-    echo "$sss$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
+test_expect_success 'lines with spaces at the beginning should be unchanged' '
+	echo "$sss$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
 
-    echo "$sss$sss$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
+	echo "$sss$sss$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
 
-    echo "$sss$sss$sss$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual
+	echo "$sss$sss$sss$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'lines with intermediate spaces should be unchanged' '
-    echo "$ttt$sss$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual &&
+test_expect_success 'lines with intermediate spaces should be unchanged' '
+	echo "$ttt$sss$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt$sss$sss$ttt" >expect &&
-    git stripspace <expect >actual &&
-    test_cmp expect actual
+	echo "$ttt$sss$sss$ttt" >expect &&
+	git stripspace <expect >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'consecutive blank lines should be unified' '
-    printf "$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'consecutive blank lines should be unified' '
+	printf "$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt$ttt$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt$ttt$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt\n\n\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt$ttt\n" > expect &&
-    printf "$ttt\n\n\n\n\n$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt$ttt\n" > expect &&
+	printf "$ttt\n\n\n\n\n$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt$ttt$ttt\n" > expect &&
-    printf "$ttt\n\n\n\n\n$ttt$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt$ttt$ttt\n" > expect &&
+	printf "$ttt\n\n\n\n\n$ttt$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt$ttt$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt$ttt$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt\n" > expect &&
-    printf "$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt\n" > expect &&
+	printf "$ttt\n\t\n \n\n  \t\t\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt$ttt\n" > expect &&
-    printf "$ttt\n\t\n \n\n  \t\t\n$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$ttt$ttt\n" > expect &&
+	printf "$ttt\n\t\n \n\n  \t\t\n$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$ttt$ttt$ttt\n" > expect &&
-    printf "$ttt\n\t\n \n\n  \t\t\n$ttt$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual
+	printf "$ttt\n\n$ttt$ttt$ttt\n" > expect &&
+	printf "$ttt\n\t\n \n\n  \t\t\n$ttt$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'only consecutive blank lines should be completely removed' '
+test_expect_success 'only consecutive blank lines should be completely removed' '
 
-    printf "\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "\n\n\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "\n\n\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss\n$sss\n$sss\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "$sss\n$sss\n$sss\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss$sss\n$sss\n\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "$sss$sss\n$sss\n\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "\n$sss\n$sss$sss\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "\n$sss\n$sss$sss\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss$sss$sss$sss\n\n\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "$sss$sss$sss$sss\n\n\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "\n$sss$sss$sss$sss\n\n" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "\n$sss$sss$sss$sss\n\n" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "\n\n$sss$sss$sss$sss\n" | git stripspace >actual &&
-    test_must_be_empty actual
+	printf "\n\n$sss$sss$sss$sss\n" | git stripspace >actual &&
+	test_must_be_empty actual
 '
 
-test_expect_success \
-    'consecutive blank lines at the beginning should be removed' '
-    printf "$ttt\n" > expect &&
-    printf "\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'consecutive blank lines at the beginning should be removed' '
+	printf "$ttt\n" > expect &&
+	printf "\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" > expect &&
-    printf "\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n" > expect &&
+	printf "\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n" > expect &&
-    printf "\n\n\n$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n" > expect &&
+	printf "\n\n\n$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt\n" > expect &&
-    printf "\n\n\n$ttt$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt\n" > expect &&
+	printf "\n\n\n$ttt$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt$ttt\n" > expect &&
-    printf "\n\n\n$ttt$ttt$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt$ttt\n" > expect &&
+	printf "\n\n\n$ttt$ttt$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" > expect &&
+	printf "$ttt\n" > expect &&
 
-    printf "$sss\n$sss\n$sss\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$sss\n$sss\n$sss\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "\n$sss\n$sss$sss\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "\n$sss\n$sss$sss\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$sss$sss\n$sss\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$sss$sss\n$sss\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$sss$sss$sss\n\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$sss$sss$sss\n\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "\n$sss$sss$sss\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "\n$sss$sss$sss\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "\n\n$sss$sss$sss\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual
+	printf "\n\n$sss$sss$sss\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'consecutive blank lines at the end should be removed' '
-    printf "$ttt\n" > expect &&
-    printf "$ttt\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'consecutive blank lines at the end should be removed' '
+	printf "$ttt\n" > expect &&
+	printf "$ttt\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" > expect &&
-    printf "$ttt\n\n\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n" > expect &&
+	printf "$ttt\n\n\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n" > expect &&
-    printf "$ttt$ttt\n\n\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n" > expect &&
+	printf "$ttt$ttt\n\n\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt\n" > expect &&
-    printf "$ttt$ttt$ttt\n\n\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt\n" > expect &&
+	printf "$ttt$ttt$ttt\n\n\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt$ttt\n" > expect &&
-    printf "$ttt$ttt$ttt$ttt\n\n\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt$ttt$ttt\n" > expect &&
+	printf "$ttt$ttt$ttt$ttt\n\n\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" > expect &&
+	printf "$ttt\n" > expect &&
 
-    printf "$ttt\n$sss\n$sss\n$sss\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$sss\n$sss\n$sss\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$sss\n$sss$sss\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$sss\n$sss$sss\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$sss$sss\n$sss\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$sss$sss\n$sss\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$sss$sss$sss\n\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$sss$sss$sss\n\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n$sss$sss$sss\n\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n\n$sss$sss$sss\n\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n\n\n$sss$sss$sss\n" | git stripspace >actual &&
-    test_cmp expect actual
+	printf "$ttt\n\n\n$sss$sss$sss\n" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'text without newline at end should end with newline' '
-    test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
+test_expect_success 'text without newline at end should end with newline' '
+	test $(printf "$ttt" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt$ttt$ttt" | git stripspace | wc -l) -gt 0
 '
 
 # text plus spaces at the end:
 
-test_expect_success \
-    'text plus spaces without newline at end should end with newline' '
-    test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
-    test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
+test_expect_success 'text plus spaces without newline at end should end with newline' '
+	test $(printf "$ttt$sss" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt$ttt$sss" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$ttt$sss$sss" | git stripspace | wc -l) -gt 0 &&
+	test $(printf "$ttt$sss$sss$sss" | git stripspace | wc -l) -gt 0
 '
 
-test_expect_success \
-    'text plus spaces without newline at end should not show spaces' '
-    ! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+test_expect_success 'text plus spaces without newline at end should not show spaces' '
+	! (printf "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (printf "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (printf "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (printf "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (printf "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (printf "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
 '
 
-test_expect_success \
-    'text plus spaces without newline should show the correct lines' '
-    printf "$ttt\n" >expect &&
-    printf "$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'text plus spaces without newline should show the correct lines' '
+	printf "$ttt\n" >expect &&
+	printf "$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" >expect &&
-    printf "$ttt$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n" >expect &&
+	printf "$ttt$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n" >expect &&
-    printf "$ttt$sss$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n" >expect &&
+	printf "$ttt$sss$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n" >expect &&
-    printf "$ttt$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n" >expect &&
+	printf "$ttt$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n" >expect &&
-    printf "$ttt$ttt$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n" >expect &&
+	printf "$ttt$ttt$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt$ttt\n" >expect &&
-    printf "$ttt$ttt$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual
+	printf "$ttt$ttt$ttt\n" >expect &&
+	printf "$ttt$ttt$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
-test_expect_success \
-    'text plus spaces at end should not show spaces' '
-    ! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
-    ! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
+test_expect_success 'text plus spaces at end should not show spaces' '
+	! (echo "$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (echo "$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (echo "$ttt$ttt$ttt$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (echo "$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (echo "$ttt$ttt$sss$sss" | git stripspace | grep "  " >/dev/null) &&
+	! (echo "$ttt$sss$sss$sss" | git stripspace | grep "  " >/dev/null)
 '
 
-test_expect_success \
-    'text plus spaces at end should be cleaned and newline must remain' '
-    echo "$ttt" >expect &&
-    echo "$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'text plus spaces at end should be cleaned and newline must remain' '
+	echo "$ttt" >expect &&
+	echo "$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt" >expect &&
-    echo "$ttt$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	echo "$ttt" >expect &&
+	echo "$ttt$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt" >expect &&
-    echo "$ttt$sss$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	echo "$ttt" >expect &&
+	echo "$ttt$sss$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt$ttt" >expect &&
-    echo "$ttt$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	echo "$ttt$ttt" >expect &&
+	echo "$ttt$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt$ttt" >expect &&
-    echo "$ttt$ttt$sss$sss" | git stripspace >actual &&
-    test_cmp expect actual &&
+	echo "$ttt$ttt" >expect &&
+	echo "$ttt$ttt$sss$sss" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    echo "$ttt$ttt$ttt" >expect &&
-    echo "$ttt$ttt$ttt$sss" | git stripspace >actual &&
-    test_cmp expect actual
+	echo "$ttt$ttt$ttt" >expect &&
+	echo "$ttt$ttt$ttt$sss" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
 # spaces only:
 
-test_expect_success \
-    'spaces with newline at end should be replaced with empty string' '
-    echo | git stripspace >actual &&
-    test_must_be_empty actual &&
+test_expect_success 'spaces with newline at end should be replaced with empty string' '
+	echo | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    echo "$sss" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	echo "$sss" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    echo "$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	echo "$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    echo "$sss$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	echo "$sss$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    echo "$sss$sss$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual
+	echo "$sss$sss$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual
 '
 
-test_expect_success \
-    'spaces without newline at end should not show spaces' '
-    ! (printf "" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
-    ! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
+test_expect_success 'spaces without newline at end should not show spaces' '
+	! (printf "" | git stripspace | grep " " >/dev/null) &&
+	! (printf "$sss" | git stripspace | grep " " >/dev/null) &&
+	! (printf "$sss$sss" | git stripspace | grep " " >/dev/null) &&
+	! (printf "$sss$sss$sss" | git stripspace | grep " " >/dev/null) &&
+	! (printf "$sss$sss$sss$sss" | git stripspace | grep " " >/dev/null)
 '
 
-test_expect_success \
-    'spaces without newline at end should be replaced with empty string' '
-    printf "" | git stripspace >actual &&
-    test_must_be_empty actual &&
+test_expect_success 'spaces without newline at end should be replaced with empty string' '
+	printf "" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual &&
+	printf "$sss$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual &&
 
-    printf "$sss$sss$sss$sss" | git stripspace >actual &&
-    test_must_be_empty actual
+	printf "$sss$sss$sss$sss" | git stripspace >actual &&
+	test_must_be_empty actual
 '
 
-test_expect_success \
-    'consecutive text lines should be unchanged' '
-    printf "$ttt$ttt\n$ttt\n" >expect &&
-    printf "$ttt$ttt\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+test_expect_success 'consecutive text lines should be unchanged' '
+	printf "$ttt$ttt\n$ttt\n" >expect &&
+	printf "$ttt$ttt\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$ttt$ttt\n$ttt\n" >expect &&
-    printf "$ttt\n$ttt$ttt\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$ttt$ttt\n$ttt\n" >expect &&
+	printf "$ttt\n$ttt$ttt\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$ttt\n$ttt\n$ttt$ttt\n" >expect &&
-    printf "$ttt\n$ttt\n$ttt\n$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$ttt\n$ttt\n$ttt$ttt\n" >expect &&
+	printf "$ttt\n$ttt\n$ttt\n$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$ttt\n\n$ttt$ttt\n$ttt\n" >expect &&
-    printf "$ttt\n$ttt\n\n$ttt$ttt\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt\n$ttt\n\n$ttt$ttt\n$ttt\n" >expect &&
+	printf "$ttt\n$ttt\n\n$ttt$ttt\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt$ttt\n\n$ttt\n$ttt$ttt\n" >expect &&
-    printf "$ttt$ttt\n\n$ttt\n$ttt$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual &&
+	printf "$ttt$ttt\n\n$ttt\n$ttt$ttt\n" >expect &&
+	printf "$ttt$ttt\n\n$ttt\n$ttt$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual &&
 
-    printf "$ttt\n$ttt$ttt\n\n$ttt\n" >expect &&
-    printf "$ttt\n$ttt$ttt\n\n$ttt\n" | git stripspace >actual &&
-    test_cmp expect actual
+	printf "$ttt\n$ttt$ttt\n\n$ttt\n" >expect &&
+	printf "$ttt\n$ttt$ttt\n\n$ttt\n" | git stripspace >actual &&
+	test_cmp expect actual
 '
 
 test_expect_success 'strip comments, too' '
-- 
2.19.0.444.g18242da7ef-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] t7001: reformat to newer style
  2018-09-21 23:58 ` [PATCH 1/3] t7001: reformat " Stefan Beller
@ 2018-09-24 13:31   ` Derrick Stolee
  2018-09-24 16:09     ` Junio C Hamano
  2018-09-24 18:51     ` Stefan Beller
  0 siblings, 2 replies; 8+ messages in thread
From: Derrick Stolee @ 2018-09-24 13:31 UTC (permalink / raw)
  To: Stefan Beller, git

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 <stolee@gmail.com>
> Reported-by: Jeff Hostetler <git@jeffhostetler.com>
> Signed-off-by: Stefan Beller <sbeller@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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] t7001: reformat to newer style
  2018-09-24 13:31   ` Derrick Stolee
@ 2018-09-24 16:09     ` Junio C Hamano
  2018-09-24 18:51     ` Stefan Beller
  1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2018-09-24 16:09 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Stefan Beller, git

Derrick Stolee <stolee@gmail.com> writes:

>> +test_expect_success 'moving the file back into subdirectory' '
>> +	cd path0 && git mv ../path1/COPYING COPYING
>> +'
>
> Split at &&, use subshell?

Yes, I was almost going to point out the same, saying "'reformat to
newer style' is much larger than only changing how the test body is
formatted", but was debating myself, as a good "modernization patch"
needs both mechanical changes and manual/semantic clean-ups, and it
is very clear that these three patches deliberately limit themselves
to the former for easier verification.

It is relatively rare that files are not touched by any in-flight
topic in the codebase, which is a good opportunity to apply this
kind of wholesale clean-up, so I tend to agree that it is a shame
not to do the non-mechanical clean up in the same series.  Perhaps
the best way would be to keep these three mechanical steps as they
are, and then follow-up with non-mechanical clean-up like you
suggested.

>> +test_expect_success 'commiting the change' '
>> +	cd .. && git commit -m move-in -a
>> +'
>
> Drop "cd .." (and the comments about being in path0)

... when the previous step moves to "git -C path0 mv ..." or
preferrably "(cd path0 && git mv ...)".


> [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 &&.

Create an empty file by redirecting the output from a no-op into it,
and then adding the result.  I agree with you that this would be
easier to read on two lines.

>> +	git mv path1/path0/ path2/ &&
>> +	test_path_is_dir path2/path0/
>> +'

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] t7001: reformat to newer style
  2018-09-24 13:31   ` Derrick Stolee
  2018-09-24 16:09     ` Junio C Hamano
@ 2018-09-24 18:51     ` Stefan Beller
  2018-09-25 20:36       ` Junio C Hamano
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Beller @ 2018-09-24 18:51 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git

On Mon, Sep 24, 2018 at 6:31 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> 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.

Heh, thanks for calling that out. So we're looking at a full formatter
instead of a partial formatter that helps moving in the right direction now. :-/

I would prefer to use automation as much as possible for these tasks
to keep it easy to apply it at scale once a file is not touched by
master..pu any more.

When applying styles manually, there is sometimes a judgement call,
which would like to the inevitable bikeshedding that I'd prefer to avoid.

> > +test_expect_success 'moving the file out of subdirectory' '
> > +     cd path0 && git mv COPYING ../path1/COPYING
> > +'
> Perhaps split this line on the &&?

In real modern testing, this could also be

    git -C path0 mv ...

which would also fix the cd.. below and not needing
a subshell there either (using -C again).

Looking at this from a higher level, nowadays I would write
tests that have more lines in them, instead of having
one git command per test.

> > +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 &&.

Yes.

> > +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.

Okay, I'll go manually over these tests to adapt to new style.

Thanks for looking over the patch!
Stefan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] t7001: reformat to newer style
  2018-09-24 18:51     ` Stefan Beller
@ 2018-09-25 20:36       ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2018-09-25 20:36 UTC (permalink / raw)
  To: Stefan Beller; +Cc: Derrick Stolee, git

Stefan Beller <sbeller@google.com> writes:

> Heh, thanks for calling that out. So we're looking at a full formatter
> instead of a partial formatter that helps moving in the right direction now. :-/

The parts left out of these patches (e.g. use subshell when working
in a subdirectory) need human decision and a bit of good taste, and
I think it was a good design decision to keep these three patches
all mechanical.  There do need a follow-up [PATCH {4,5,6}/3] that
cannot be done mechanically, though, before the result can be called
"this script has been cleaned up and new people are encouraged to
mimick the way it does things".  

That way, we can keep the mechanically good bits that are early in
the series while polishing the later bits that need human judgement.



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-09-25 20:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-21 23:58 [PATCH 0/3] bring some tests to newer style Stefan Beller
2018-09-21 23:58 ` [PATCH 1/3] t7001: reformat " Stefan Beller
2018-09-24 13:31   ` Derrick Stolee
2018-09-24 16:09     ` Junio C Hamano
2018-09-24 18:51     ` Stefan Beller
2018-09-25 20:36       ` Junio C Hamano
2018-09-21 23:58 ` [PATCH 2/3] t7004: reformat style Stefan Beller
2018-09-21 23:58 ` [PATCH 3/3] t0030: " Stefan Beller

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).