{"thread":{"id":"42966","subject":"[PATCH v2 2/3] Make test t3700-add.sh more robust","startedAt":"2016-07-29T21:19:29Z","lastAt":"2016-07-29T22:03:51Z","messageCount":2,"participants":["Ingo Brückl","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":3},"messages":[{"id":"292562","messageId":"579bc6ca.3f2601c7.bm001@wupperonline.de","threadId":"42966","inReplyTo":null,"subject":"[PATCH v2 2/3] Make test t3700-add.sh more robust","fromName":"Ingo Brückl","fromEmail":"ib@wupperonline.de","sentAt":"2016-07-29T21:11:36Z","receivedAt":"2016-07-29T21:19:29Z","isPatch":true,"sender":{"key":"ib@wupperonline.de","avatar":"https://avatars.githubusercontent.com/u/123327?v=4"},"body":"Don't rely on chmod to work on the underlying platform (although it\nwouldn't harm the result of the '--chmod=-x' test). Directly check the\nresult of the --chmod option.\n\nAdd a test_mode_in_index helper function in order to check for success.\n\nSigned-off-by: Ingo Brückl <ib@wupperonline.de>\n---\n t/t3700-add.sh          | 20 ++++----------------\n t/test-lib-functions.sh | 14 ++++++++++++++\n 2 files changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex 494f5b8..c08ec9e 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -332,25 +332,13 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out\n \ttest_i18ncmp expect.err actual.err\n '\n\n-test_expect_success 'git add --chmod=+x stages a non-executable file with +x' '\n+test_expect_success 'git add --chmod=[+-]x stages correctly' '\n \trm -f foo1 &&\n \techo foo >foo1 &&\n \tgit add --chmod=+x foo1 &&\n-\tcase \"$(git ls-files --stage foo1)\" in\n-\t100755\" \"*foo1) echo pass;;\n-\t*) echo fail; git ls-files --stage foo1; (exit 1);;\n-\tesac\n-'\n-\n-test_expect_success 'git add --chmod=-x stages an executable file with -x' '\n-\trm -f xfoo1 &&\n-\techo foo >xfoo1 &&\n-\tchmod 755 xfoo1 &&\n-\tgit add --chmod=-x xfoo1 &&\n-\tcase \"$(git ls-files --stage xfoo1)\" in\n-\t100644\" \"*xfoo1) echo pass;;\n-\t*) echo fail; git ls-files --stage xfoo1; (exit 1);;\n-\tesac\n+\ttest_mode_in_index 100755 foo1 &&\n+\tgit add --chmod=-x foo1 &&\n+\ttest_mode_in_index 100644 foo1\n '\n\n test_expect_success POSIXPERM,SYMLINKS 'git add --chmod=+x with symlinks' '\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 4f7eadb..0e6652b 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -990,3 +990,17 @@ test_copy_bytes () {\n \t\t}\n \t' - \"$1\"\n }\n+\n+# Test the file mode \"$1\" of the file \"$2\" in the index.\n+test_mode_in_index () {\n+\tcase \"$(git ls-files --stage \"$2\")\" in\n+\t\t$1\\ *\"$2\")\n+\t\t\techo pass\n+\t\t\t;;\n+\t\t*)\n+\t\t\techo fail\n+\t\t\tgit ls-files --stage \"$2\"\n+\t\t\treturn 1\n+\t\t\t;;\n+\tesac\n+}\n--\n2.9.2\n\n"},{"id":"292566","messageId":"xmqq1t2cm79c.fsf@gitster.mtv.corp.google.com","threadId":"42966","inReplyTo":"579bc6ca.3f2601c7.bm001@wupperonline.de","subject":"Re: [PATCH v2 2/3] Make test t3700-add.sh more robust","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-29T22:03:43Z","receivedAt":"2016-07-29T22:03:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ingo Brückl <ib@wupperonline.de> writes:\n\n> Subject: Re: [PATCH v2 2/3] Make test t3700-add.sh more robust\n\nPlease check output from \"git shortlog --no-merges -100\" to see how\nyour titles play well with others.  We typically prefix the title\nwith a specific area, a colon, and a sentence that does not begin in\na capital letter and does not end with a full-stop.\n\n> Don't rely on chmod to work on the underlying platform (although it\n> wouldn't harm the result of the '--chmod=-x' test). Directly check the\n> result of the --chmod option.\n>\n> Add a test_mode_in_index helper function in order to check for success.\n\nHmph, I do not immediately see the point of having the helper in\ntest-lib-functions.sh, though.  This helper looks more or less\nspecific to this test script.\n\nIn any case, I think addition of the \"test_mode_in_index\" helper and\nconversion from case/esac to the helper should be a separate patch\nfrom what this patch wants to do, which is to merge two more-or-less\nredundant tests into one.\n\nThanks.\n\n"}]}