{"thread":{"id":"25071","subject":"[PATCH] t7003: Use test_commit instead of custom function","startedAt":"2010-09-10T22:06:36Z","lastAt":"2010-09-13T15:46:13Z","messageCount":4,"participants":["Brian Gernhardt","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"150480","messageId":"1284156396-81023-1-git-send-email-brian@gernhardtsoftware.com","threadId":"25071","inReplyTo":null,"subject":"[PATCH] t7003: Use test_commit instead of custom function","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-09-10T22:06:36Z","receivedAt":"2010-09-10T22:06:36Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"t7003-filter-branch.sh had a make_commit() function that was identical\nto test_commit() in test-lib.sh except that it used tr to create a\nlowercase file name from the uppercase branch name instead of\nappending \".t\".\n\nNot only is this unneeded code duplication, it also was something\nsimply waiting to fail on case-insensitive file systems.  So replace\nall uses of make_commit with test_commit.\n\nWhile we're editing the setup, chain it together with && so that\nfailures early in the sequence don't get lost and add a commit graph.\n\nSigned-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>\n---\n t/t7003-filter-branch.sh |   77 ++++++++++++++++++++++++----------------------\n 1 files changed, 40 insertions(+), 37 deletions(-)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 12aa63e..e022773 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -3,31 +3,34 @@\n test_description='git filter-branch'\n . ./test-lib.sh\n \n-make_commit () {\n-\tlower=$(echo $1 | tr '[A-Z]' '[a-z]')\n-\techo $lower > $lower\n-\tgit add $lower\n-\ttest_tick\n-\tgit commit -m $1\n-\tgit tag $1\n-}\n-\n test_expect_success 'setup' '\n-\tmake_commit A\n-\tmake_commit B\n-\tgit checkout -b branch B\n-\tmake_commit D\n-\tmkdir dir\n-\tmake_commit dir/D\n-\tmake_commit E\n-\tgit checkout master\n-\tmake_commit C\n-\tgit checkout branch\n-\tgit merge C\n-\tgit tag F\n-\tmake_commit G\n-\tmake_commit H\n-'\n+\ttest_commit A &&\n+\ttest_commit B &&\n+\tgit checkout -b branch B &&\n+\ttest_commit D &&\n+\tmkdir dir &&\n+\ttest_commit dir/D &&\n+\ttest_commit E &&\n+\tgit checkout master &&\n+\ttest_commit C &&\n+\tgit checkout branch &&\n+\tgit merge C &&\n+\tgit tag F &&\n+\ttest_commit G &&\n+\ttest_commit H\n+'\n+# * (HEAD, branch) H\n+# * G\n+# *   Merge commit 'C' into branch\n+# |\\\n+# | * (master) C\n+# * | E\n+# * | dir/D\n+# * | D\n+# |/\n+# * B\n+# * A\n+\n \n H=$(git rev-parse H)\n \n@@ -65,14 +68,14 @@ test_expect_success 'Fail if commit filter fails' '\n '\n \n test_expect_success 'rewrite, renaming a specific file' '\n-\tgit filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n+\tgit filter-branch -f --tree-filter \"mv D.t doh || :\" HEAD\n '\n \n test_expect_success 'test that the file was renamed' '\n-\ttest d = \"$(git show HEAD:doh --)\" &&\n-\t! test -f d &&\n+\ttest D = \"$(git show HEAD:doh --)\" &&\n+\t! test -f D.t &&\n \ttest -f doh &&\n-\ttest d = \"$(cat doh)\"\n+\ttest D = \"$(cat doh)\"\n '\n \n test_expect_success 'rewrite, renaming a specific directory' '\n@@ -80,18 +83,18 @@ test_expect_success 'rewrite, renaming a specific directory' '\n '\n \n test_expect_success 'test that the directory was renamed' '\n-\ttest dir/d = \"$(git show HEAD:diroh/d --)\" &&\n+\ttest dir/D = \"$(git show HEAD:diroh/D.t --)\" &&\n \t! test -d dir &&\n \ttest -d diroh &&\n \t! test -d diroh/dir &&\n-\ttest -f diroh/d &&\n-\ttest dir/d = \"$(cat diroh/d)\"\n+\ttest -f diroh/D.t &&\n+\ttest dir/D = \"$(cat diroh/D.t)\"\n '\n \n git tag oldD HEAD~4\n test_expect_success 'rewrite one branch, keeping a side branch' '\n \tgit branch modD oldD &&\n-\tgit filter-branch -f --tree-filter \"mv b boh || :\" D..modD\n+\tgit filter-branch -f --tree-filter \"mv B.t boh || :\" D..modD\n '\n \n test_expect_success 'common ancestor is still common (unchanged)' '\n@@ -104,13 +107,13 @@ test_expect_success 'filter subdirectory only' '\n \tgit add subdir/new &&\n \ttest_tick &&\n \tgit commit -m \"subdir\" &&\n-\techo H > a &&\n+\techo H > A.t &&\n \ttest_tick &&\n-\tgit commit -m \"not subdir\" a &&\n+\tgit commit -m \"not subdir\" A.t &&\n \techo A > subdir/new &&\n \ttest_tick &&\n \tgit commit -m \"again subdir\" subdir/new &&\n-\tgit rm a &&\n+\tgit rm A.t &&\n \ttest_tick &&\n \tgit commit -m \"again not subdir\" &&\n \tgit branch sub &&\n@@ -134,7 +137,7 @@ test_expect_success 'more setup' '\n \tgit add subdir/new &&\n \ttest_tick &&\n \tgit commit -m \"subdir on master\" subdir/new &&\n-\tgit rm a &&\n+\tgit rm A.t &&\n \ttest_tick &&\n \tgit commit -m \"again subdir on master\" &&\n \tgit merge branch\n@@ -283,8 +286,8 @@ test_expect_success 'Tag name filtering allows slashes in tag names' '\n \n test_expect_success 'Prune empty commits' '\n \tgit rev-list HEAD > expect &&\n-\tmake_commit to_remove &&\n-\tgit filter-branch -f --index-filter \"git update-index --remove to_remove\" --prune-empty HEAD &&\n+\ttest_commit to_remove &&\n+\tgit filter-branch -f --index-filter \"git update-index --remove to_remove.t\" --prune-empty HEAD &&\n \tgit rev-list HEAD > actual &&\n \ttest_cmp expect actual\n '\n-- \n1.7.2.3.531.g1a508\n"},{"id":"150570","messageId":"214E9A5B-E9CE-4B98-88EC-744DBE1F3C32@gernhardtsoftware.com","threadId":"25071","inReplyTo":"1284156396-81023-1-git-send-email-brian@gernhardtsoftware.com","subject":"Re: [PATCH] t7003: Use test_commit instead of custom function","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2010-09-13T04:14:25Z","receivedAt":"2010-09-13T04:14:25Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Sep 10, 2010, at 6:06 PM, Brian Gernhardt wrote:\n\n> Not only is this unneeded code duplication, it also was something\n> simply waiting to fail on case-insensitive file systems.  So replace\n> all uses of make_commit with test_commit.\n\nTo 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'\n\nHowever, the patch applies to master or maint.\n\n~~ Brian"},{"id":"150586","messageId":"AANLkTi=1p7tTNfhX7_4vMPFGtRC=F14im3qo009RPJV9@mail.gmail.com","threadId":"25071","inReplyTo":"214E9A5B-E9CE-4B98-88EC-744DBE1F3C32@gernhardtsoftware.com","subject":"Re: [PATCH] t7003: Use test_commit instead of custom function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-13T12:11:32Z","receivedAt":"2010-09-13T12:11:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Sep 13, 2010 at 04:14, Brian Gernhardt\n<brian@gernhardtsoftware.com> wrote:\n>\n> On Sep 10, 2010, at 6:06 PM, Brian Gernhardt wrote:\n>\n>> Not only is this unneeded code duplication, it also was something\n>> simply waiting to fail on case-insensitive file systems.  So replace\n>> all uses of make_commit with test_commit.\n>\n> 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'\n>\n> However, the patch applies to master or maint.\n\nAnd you've modified the casing so it passes? Maybe it should skip on\ncase-insensitive systems instead, or perhaps we should modify it and\ntest for that elsewhere.\n"},{"id":"150602","messageId":"AANLkTikqGLvMHzsa4We7OUcYz5o2v62huO1OhiYLPHeS@mail.gmail.com","threadId":"25071","inReplyTo":"alpine.DEB.1.00.1009131741270.2439@intel-tinevez-2-302","subject":"Re: [PATCH] t7003: Use test_commit instead of custom function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-13T15:46:13Z","receivedAt":"2010-09-13T15:46:13Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Sep 13, 2010 at 15:44, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Mon, 13 Sep 2010, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Mon, Sep 13, 2010 at 15:24, Brian Gernhardt\n>> <brian@gernhardtsoftware.com> wrote:\n>> > On Sep 13, 2010, at 8:11 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>>\n>> > In addition, why do we need a custom make_commit when we have\n>> > test_commit in test-lib.sh?\n>>\n>> It was added in v1.5.3-rc0~69^2~10 by Johannes Schindelin (CC'd). It's\n>> probably just now-needless duplication, but I haven't familirized myself\n>> with the code.\n>\n> Yep, I did not make a cleanup patch after adding test_commit. Neither did\n> I consolidate other cases in t/ where the steps are done explicitly.\n\nThanks. I didn't check when test_commit was added, it makes sense now.\n\nThere's a lot of old spiderwebs like that in t/* that could now use\ncleanup.\n"}]}