[PATCH] t7003: Use test_commit instead of custom function

[PATCH] t7003: Use test_commit instead of custom function

[Brian Gernhardt]

t7003-filter-branch.sh had a make_commit() function that was identical
to test_commit() in test-lib.sh except that it used tr to create a
lowercase file name from the uppercase branch name instead of
appending ".t".

Not only is this unneeded code duplication, it also was something
simply waiting to fail on case-insensitive file systems.  So replace
all uses of make_commit with test_commit.

While we're editing the setup, chain it together with && so that
failures early in the sequence don't get lost and add a commit graph.

Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
---
 t/t7003-filter-branch.sh |   77 ++++++++++++++++++++++++----------------------
 1 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 12aa63e..e022773 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -3,31 +3,34 @@
 test_description='git filter-branch'
 . ./test-lib.sh
 
-make_commit () {
-	lower=$(echo $1 | tr '[A-Z]' '[a-z]')
-	echo $lower > $lower
-	git add $lower
-	test_tick
-	git commit -m $1
-	git tag $1
-}
-
 test_expect_success 'setup' '
-	make_commit A
-	make_commit B
-	git checkout -b branch B
-	make_commit D
-	mkdir dir
-	make_commit dir/D
-	make_commit E
-	git checkout master
-	make_commit C
-	git checkout branch
-	git merge C
-	git tag F
-	make_commit G
-	make_commit H
-'
+	test_commit A &&
+	test_commit B &&
+	git checkout -b branch B &&
+	test_commit D &&
+	mkdir dir &&
+	test_commit dir/D &&
+	test_commit E &&
+	git checkout master &&
+	test_commit C &&
+	git checkout branch &&
+	git merge C &&
+	git tag F &&
+	test_commit G &&
+	test_commit H
+'
+# * (HEAD, branch) H
+# * G
+# *   Merge commit 'C' into branch
+# |\
+# | * (master) C
+# * | E
+# * | dir/D
+# * | D
+# |/
+# * B
+# * A
+
 
 H=$(git rev-parse H)
 
@@ -65,14 +68,14 @@ test_expect_success 'Fail if commit filter fails' '
 '
 
 test_expect_success 'rewrite, renaming a specific file' '
-	git filter-branch -f --tree-filter "mv d doh || :" HEAD
+	git filter-branch -f --tree-filter "mv D.t doh || :" HEAD
 '
 
 test_expect_success 'test that the file was renamed' '
-	test d = "$(git show HEAD:doh --)" &&
-	! test -f d &&
+	test D = "$(git show HEAD:doh --)" &&
+	! test -f D.t &&
 	test -f doh &&
-	test d = "$(cat doh)"
+	test D = "$(cat doh)"
 '
 
 test_expect_success 'rewrite, renaming a specific directory' '
@@ -80,18 +83,18 @@ test_expect_success 'rewrite, renaming a specific directory' '
 '
 
 test_expect_success 'test that the directory was renamed' '
-	test dir/d = "$(git show HEAD:diroh/d --)" &&
+	test dir/D = "$(git show HEAD:diroh/D.t --)" &&
 	! test -d dir &&
 	test -d diroh &&
 	! test -d diroh/dir &&
-	test -f diroh/d &&
-	test dir/d = "$(cat diroh/d)"
+	test -f diroh/D.t &&
+	test dir/D = "$(cat diroh/D.t)"
 '
 
 git tag oldD HEAD~4
 test_expect_success 'rewrite one branch, keeping a side branch' '
 	git branch modD oldD &&
-	git filter-branch -f --tree-filter "mv b boh || :" D..modD
+	git filter-branch -f --tree-filter "mv B.t boh || :" D..modD
 '
 
 test_expect_success 'common ancestor is still common (unchanged)' '
@@ -104,13 +107,13 @@ test_expect_success 'filter subdirectory only' '
 	git add subdir/new &&
 	test_tick &&
 	git commit -m "subdir" &&
-	echo H > a &&
+	echo H > A.t &&
 	test_tick &&
-	git commit -m "not subdir" a &&
+	git commit -m "not subdir" A.t &&
 	echo A > subdir/new &&
 	test_tick &&
 	git commit -m "again subdir" subdir/new &&
-	git rm a &&
+	git rm A.t &&
 	test_tick &&
 	git commit -m "again not subdir" &&
 	git branch sub &&
@@ -134,7 +137,7 @@ test_expect_success 'more setup' '
 	git add subdir/new &&
 	test_tick &&
 	git commit -m "subdir on master" subdir/new &&
-	git rm a &&
+	git rm A.t &&
 	test_tick &&
 	git commit -m "again subdir on master" &&
 	git merge branch
@@ -283,8 +286,8 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '
 
 test_expect_success 'Prune empty commits' '
 	git rev-list HEAD > expect &&
-	make_commit to_remove &&
-	git filter-branch -f --index-filter "git update-index --remove to_remove" --prune-empty HEAD &&
+	test_commit to_remove &&
+	git filter-branch -f --index-filter "git update-index --remove to_remove.t" --prune-empty HEAD &&
 	git rev-list HEAD > actual &&
 	test_cmp expect actual
 '
-- 
1.7.2.3.531.g1a508

Re: [PATCH] t7003: Use test_commit instead of custom function

[Brian Gernhardt]

On Sep 10, 2010, at 6:06 PM, Brian Gernhardt wrote:

> Not only is this unneeded code duplication, it also was something
> simply waiting to fail on case-insensitive file systems.  So replace
> all uses of make_commit with test_commit.

To be more specific: this causes a failure in next on case-insensitive HFS+.  The commit that finally triggered the problem is 7ec344d: "filter-branch: retire --remap-to-ancestor", by using `git reset --hard A` which got confused on if 'A' was 'refs/branches/A' or './a'

However, the patch applies to master or maint.

~~ Brian

Re: [PATCH] t7003: Use test_commit instead of custom function

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 13, 2010 at 04:14, Brian Gernhardt
<brian@gernhardtsoftware.com> wrote:
>
> On Sep 10, 2010, at 6:06 PM, Brian Gernhardt wrote:
>
>> Not only is this unneeded code duplication, it also was something
>> simply waiting to fail on case-insensitive file systems.  So replace
>> all uses of make_commit with test_commit.
>
> To be more specific: this causes a failure in next on case-insensitive HFS+.  The commit that finally triggered the problem is 7ec344d: "filter-branch: retire --remap-to-ancestor", by using `git reset --hard A` which got confused on if 'A' was 'refs/branches/A' or './a'
>
> However, the patch applies to master or maint.

And you've modified the casing so it passes? Maybe it should skip on
case-insensitive systems instead, or perhaps we should modify it and
test for that elsewhere.

Re: [PATCH] t7003: Use test_commit instead of custom function

[Ævar Arnfjörð Bjarmason]

On Mon, Sep 13, 2010 at 15:44, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 13 Sep 2010, Ævar Arnfjörð Bjarmason wrote:
>
>> On Mon, Sep 13, 2010 at 15:24, Brian Gernhardt
>> <brian@gernhardtsoftware.com> wrote:
>> > On Sep 13, 2010, at 8:11 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>
>> > In addition, why do we need a custom make_commit when we have
>> > test_commit in test-lib.sh?
>>
>> It was added in v1.5.3-rc0~69^2~10 by Johannes Schindelin (CC'd). It's
>> probably just now-needless duplication, but I haven't familirized myself
>> with the code.
>
> Yep, I did not make a cleanup patch after adding test_commit. Neither did
> I consolidate other cases in t/ where the steps are done explicitly.

Thanks. I didn't check when test_commit was added, it makes sense now.

There's a lot of old spiderwebs like that in t/* that could now use
cleanup.