{"thread":{"id":"53134","subject":"[PATCH] t4200-rerere.sh: use test_path_is_* functions in the script","startedAt":"2020-03-30T17:16:31Z","lastAt":"2020-04-09T18:09:48Z","messageCount":4,"participants":["Allo via GitGitGadget","Danh Doan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"394382","messageId":"pull.595.git.1585588586605.gitgitgadget@gmail.com","threadId":"53134","inReplyTo":null,"subject":"[PATCH] t4200-rerere.sh: use test_path_is_* functions in the script","fromName":"Allo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-30T17:16:26Z","receivedAt":"2020-03-30T17:16:31Z","isPatch":true,"sender":{"key":"name:Allo","avatar":null},"body":"From: Angel Pan <dinoallosaurus1111@gmail.com>\n\nThis is a microproject for GSoC.\nThe original code works fine. However, since the code base has already have help-functions(in test-lib-functions),\nthe change is able to help people understand the code better, as well as improve debug-friendliness.\n\nSigned-off-by: Angel Pan <dinoallosaurus1111@gmail.com>\n---\n    [GSoC/PATCH] t4200-rerere.sh: use test_path_is_* functions in the script\n    \n    Hello again. I have sent a mail here: last mail\n    [https://lore.kernel.org/git/CAFk=nY7efz8ZisEuyAcU09J+ja4qE8LorUUPeupGVEjBBc6HDA@mail.gmail.com/T/#u]\n    After all, I decided to replace test -(e|f|d) with test_path_is_*\n    functions as my microproject for GSoC. I am looking forward to working\n    with every contributor here. I also have another question: For a commit\n    like this involving 10+ insertions, would it be better if I\n    seperate-commited all of them? But this kind of change seems trivial...\n    \n    Signed-off-by: Angel Pan dinoallosaurus1111@gmail.com\n    [dinoallosaurus1111@gmail.com]\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-595%2Fdinoallo%2Fmaster-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-595/dinoallo/master-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/595\n\n t/t4200-rerere.sh | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex 831d424c472..da8be1c6254 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -70,7 +70,7 @@ test_expect_success 'nothing recorded without rerere' '\n \trm -rf .git/rr-cache &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge first &&\n-\t! test -d .git/rr-cache\n+\t! test_path_is_dir .git/rr-cache\n '\n \n test_expect_success 'activate rerere, old style (conflicting merge)' '\n@@ -82,8 +82,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '\n \tsha1=$(perl -pe \"s/\t.*//\" .git/MERGE_RR) &&\n \trr=.git/rr-cache/$sha1 &&\n \tgrep \"^=======\\$\" $rr/preimage &&\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage\n+\t! test_path_is_file $rr/postimage &&\n+\t! test_path_is_file $rr/thisimage\n '\n \n test_expect_success 'rerere.enabled works, too' '\n@@ -108,8 +108,8 @@ test_expect_success 'set up rr-cache' '\n \n test_expect_success 'rr-cache looks sane' '\n \t# no postimage or thisimage yet\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage &&\n+\t! test_path_is_file $rr/postimage &&\n+\t! test_path_is_file $rr/thisimage &&\n \n \t# preimage has right number of lines\n \tcnt=$(sed -ne \"/^<<<<<<</,/^>>>>>>>/p\" $rr/preimage | wc -l) &&\n@@ -165,7 +165,7 @@ test_expect_success 'first postimage wins' '\n \tgit show first:a1 | sed \"s/To die: t/To die! T/\" >expect &&\n \n \tgit commit -q -a -m \"prefer first over second\" &&\n-\ttest -f $rr/postimage &&\n+\ttest_path_is_file $rr/postimage &&\n \n \toldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&\n \n@@ -188,14 +188,14 @@ test_expect_success 'rerere clear' '\n \tmv $rr/postimage .git/post-saved &&\n \techo \"$sha1\ta1\" | perl -pe \"y/\\012/\\000/\" >.git/MERGE_RR &&\n \tgit rerere clear &&\n-\t! test -d $rr\n+\t! test_path_is_dir $rr\n '\n \n test_expect_success 'leftover directory' '\n \tgit reset --hard &&\n \tmkdir -p $rr &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'missing preimage' '\n@@ -203,7 +203,7 @@ test_expect_success 'missing preimage' '\n \tmkdir -p $rr &&\n \tcp .git/post-saved $rr/postimage &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'set up for garbage collection tests' '\n@@ -228,16 +228,16 @@ test_expect_success 'set up for garbage collection tests' '\n \n test_expect_success 'gc preserves young or recently used records' '\n \tgit rerere gc &&\n-\ttest -f $rr/preimage &&\n-\ttest -f $rr2/preimage\n+\ttest_path_is_file $rr/preimage &&\n+\ttest_path_is_file $rr2/preimage\n '\n \n test_expect_success 'old records rest in peace' '\n \ttest-tool chmtime =$just_over_60_days_ago $rr/postimage &&\n \ttest-tool chmtime =$just_over_15_days_ago $rr2/preimage &&\n \tgit rerere gc &&\n-\t! test -f $rr/preimage &&\n-\t! test -f $rr2/preimage\n+\t! test_path_is_file $rr/preimage &&\n+\t! test_path_is_file $rr2/preimage\n '\n \n rerere_gc_custom_expiry_test () {\n\nbase-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775\n-- \ngitgitgadget\n"},{"id":"395127","messageId":"pull.595.v2.git.1586444061576.gitgitgadget@gmail.com","threadId":"53134","inReplyTo":"pull.595.git.1585588586605.gitgitgadget@gmail.com","subject":"[PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script","fromName":"Allo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-04-09T14:54:21Z","receivedAt":"2020-04-09T14:54:26Z","isPatch":true,"sender":{"key":"name:Allo","avatar":null},"body":"From: Angel Pan <dinoallosaurus1111@gmail.com>\n\nThe original code works fine. However, since the code base has already had help-functions(in test-lib-functions),\nthe change is able to help people understand the code better, as well as improve debug-friendliness.\n\n* t4200-rerere.sh: replace \"test -d\" with \"test_path_is_dir\"\n* t4200-rerere.sh: replace \"test -f\" with \"test_path_is_file\"\n\nSigned-off-by: Angel Pan <dinoallosaurus1111@gmail.com>\n---\n    [GSoC/PATCH] t4200-rerere.sh: use test_path_is_* functions in the script\n    \n    Hello again. I thought I needed to improve my commit message since some\n    of community members friendly reminded me last week. So I revised it\n    again. Here is my patch with new commit message. If there is still\n    something I miss, a reminder will be appreciated.\n    \n    Signed-off-by: Angel Pan dinoallosaurus1111@gmail.com\n    [dinoallosaurus1111@gmail.com]\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-595%2Fdinoallo%2Fmaster-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-595/dinoallo/master-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/595\n\nRange-diff vs v1:\n\n 1:  7e0c8749016 ! 1:  c704627a465 t4200-rerere.sh: use test_path_is_* functions in the script\n     @@ Metadata\n       ## Commit message ##\n          t4200-rerere.sh: use test_path_is_* functions in the script\n      \n     -    This is a microproject for GSoC.\n     -    The original code works fine. However, since the code base has already have help-functions(in test-lib-functions),\n     +    The original code works fine. However, since the code base has already had help-functions(in test-lib-functions),\n          the change is able to help people understand the code better, as well as improve debug-friendliness.\n      \n     +    * t4200-rerere.sh: replace \"test -d\" with \"test_path_is_dir\"\n     +    * t4200-rerere.sh: replace \"test -f\" with \"test_path_is_file\"\n     +\n          Signed-off-by: Angel Pan <dinoallosaurus1111@gmail.com>\n      \n       ## t/t4200-rerere.sh ##\n\n\n t/t4200-rerere.sh | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh\nindex 831d424c472..da8be1c6254 100755\n--- a/t/t4200-rerere.sh\n+++ b/t/t4200-rerere.sh\n@@ -70,7 +70,7 @@ test_expect_success 'nothing recorded without rerere' '\n \trm -rf .git/rr-cache &&\n \tgit config rerere.enabled false &&\n \ttest_must_fail git merge first &&\n-\t! test -d .git/rr-cache\n+\t! test_path_is_dir .git/rr-cache\n '\n \n test_expect_success 'activate rerere, old style (conflicting merge)' '\n@@ -82,8 +82,8 @@ test_expect_success 'activate rerere, old style (conflicting merge)' '\n \tsha1=$(perl -pe \"s/\t.*//\" .git/MERGE_RR) &&\n \trr=.git/rr-cache/$sha1 &&\n \tgrep \"^=======\\$\" $rr/preimage &&\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage\n+\t! test_path_is_file $rr/postimage &&\n+\t! test_path_is_file $rr/thisimage\n '\n \n test_expect_success 'rerere.enabled works, too' '\n@@ -108,8 +108,8 @@ test_expect_success 'set up rr-cache' '\n \n test_expect_success 'rr-cache looks sane' '\n \t# no postimage or thisimage yet\n-\t! test -f $rr/postimage &&\n-\t! test -f $rr/thisimage &&\n+\t! test_path_is_file $rr/postimage &&\n+\t! test_path_is_file $rr/thisimage &&\n \n \t# preimage has right number of lines\n \tcnt=$(sed -ne \"/^<<<<<<</,/^>>>>>>>/p\" $rr/preimage | wc -l) &&\n@@ -165,7 +165,7 @@ test_expect_success 'first postimage wins' '\n \tgit show first:a1 | sed \"s/To die: t/To die! T/\" >expect &&\n \n \tgit commit -q -a -m \"prefer first over second\" &&\n-\ttest -f $rr/postimage &&\n+\ttest_path_is_file $rr/postimage &&\n \n \toldmtimepost=$(test-tool chmtime --get -60 $rr/postimage) &&\n \n@@ -188,14 +188,14 @@ test_expect_success 'rerere clear' '\n \tmv $rr/postimage .git/post-saved &&\n \techo \"$sha1\ta1\" | perl -pe \"y/\\012/\\000/\" >.git/MERGE_RR &&\n \tgit rerere clear &&\n-\t! test -d $rr\n+\t! test_path_is_dir $rr\n '\n \n test_expect_success 'leftover directory' '\n \tgit reset --hard &&\n \tmkdir -p $rr &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'missing preimage' '\n@@ -203,7 +203,7 @@ test_expect_success 'missing preimage' '\n \tmkdir -p $rr &&\n \tcp .git/post-saved $rr/postimage &&\n \ttest_must_fail git merge first &&\n-\ttest -f $rr/preimage\n+\ttest_path_is_file $rr/preimage\n '\n \n test_expect_success 'set up for garbage collection tests' '\n@@ -228,16 +228,16 @@ test_expect_success 'set up for garbage collection tests' '\n \n test_expect_success 'gc preserves young or recently used records' '\n \tgit rerere gc &&\n-\ttest -f $rr/preimage &&\n-\ttest -f $rr2/preimage\n+\ttest_path_is_file $rr/preimage &&\n+\ttest_path_is_file $rr2/preimage\n '\n \n test_expect_success 'old records rest in peace' '\n \ttest-tool chmtime =$just_over_60_days_ago $rr/postimage &&\n \ttest-tool chmtime =$just_over_15_days_ago $rr2/preimage &&\n \tgit rerere gc &&\n-\t! test -f $rr/preimage &&\n-\t! test -f $rr2/preimage\n+\t! test_path_is_file $rr/preimage &&\n+\t! test_path_is_file $rr2/preimage\n '\n \n rerere_gc_custom_expiry_test () {\n\nbase-commit: 9fadedd637b312089337d73c3ed8447e9f0aa775\n-- \ngitgitgadget\n"},{"id":"395129","messageId":"20200409152134.GC1937@danh.dev","threadId":"53134","inReplyTo":"pull.595.v2.git.1586444061576.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-09T15:21:34Z","receivedAt":"2020-04-09T15:21:41Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-09 14:54:21+0000, Allo via GitGitGadget <gitgitgadget@gmail.com> wrote:\n> From: Angel Pan <dinoallosaurus1111@gmail.com>\n> \n> Subject: t4200-rerere.sh: use test_path_is_* functions in the script\n\n\"in the script\" looks too verbose to me. Because the script has been\nmentioned earlier.\n\n> The original code works fine. However, since the code base has already had help-functions(in test-lib-functions),\n\nWe wrap lines at 72 characters per line.\nAnd, missing space before open parenthese.\n\nI'm not good at wording, but I think this commit message is too\nverbose. I would say something like:\n\n\tPrefer \"test_path_is_dir\" to \"test -d\" and test_path_is_file\n\tto \"test -f\" because they're more friendly to debug.\n\nThat's not a good message, either. But, it's some what simpler and\ncleaner.\n\n> the change is able to help people understand the code better, as well as improve debug-friendliness.\n> \n> * t4200-rerere.sh: replace \"test -d\" with \"test_path_is_dir\"\n> * t4200-rerere.sh: replace \"test -f\" with \"test_path_is_file\"\n> \n\n-- \nDanh\n"},{"id":"395135","messageId":"xmqq7dyoo7j0.fsf@gitster.c.googlers.com","threadId":"53134","inReplyTo":"20200409152134.GC1937@danh.dev","subject":"Re: [PATCH v2] t4200-rerere.sh: use test_path_is_* functions in the script","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-09T18:09:39Z","receivedAt":"2020-04-09T18:09:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danh Doan <congdanhqx@gmail.com> writes:\n\n> On 2020-04-09 14:54:21+0000, Allo via GitGitGadget <gitgitgadget@gmail.com> wrote:\n>> From: Angel Pan <dinoallosaurus1111@gmail.com>\n>> ...\n> That's not a good message, either. But, it's some what simpler and\n> cleaner.\n\nThank you, Allo, for a patch, and thank you Danh for a good review.\n\n\n"}]}