{"thread":{"id":"53922","subject":"[PATCH 1/4] t7401: modernize style","startedAt":"2020-07-26T14:26:13Z","lastAt":"2020-07-26T14:26:26Z","messageCount":5,"participants":["Shourya Shukla"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"402056","messageId":"20200726142553.42545-1-shouryashukla.oo@gmail.com","threadId":"53922","inReplyTo":null,"subject":"[GSoC][PATCH 0/4] t7401: modernize, cleanup and warn","fromName":"Shourya Shukla","fromEmail":"shouryashukla.oo@gmail.com","sentAt":"2020-07-26T14:25:49Z","receivedAt":"2020-07-26T14:26:13Z","isPatch":true,"sender":{"key":"shouryashukla.oo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43680618?v=4"},"body":"Greetings,\n\nThe current phase of my GSoC project involves porting\n'git submodule summary' from shell to C. While doing so, I, along with\nmy mentors Christian and Kaartic noticed some discrepancies in the test\nscript when trying to add a couple of tests to it. Though the test works\nperfectly for my C port of 'summary', there were some unexpected behaviours\nwhen trying to some tests to it. This patch series addresses these\nissues in the test script by modernizing it, cleaning it up and warning\nabout some other issues.\n\nChiefly about patch 4/4 (t7401: add a WARNING and a NEEDSWORK),\nwhen trying to write a test for verifying the summary output of\ndeinitialized submodule, doing a 'git submodule deinit ' did not\nbear any fruit since the submodule never really got deinitialized. \nThe deinit documentation states that:\n\n\tUnregister the given submodules, i.e. remove the whole\n\tsubmodule.$name section from .git/config together with\n\ttheir work tree.\n\nSomething which was not actually happening in the test. It appeared\nthat the reason for the deinit issue is that the test script uses\n'git add' to add submodules instead of the command 'git submodule add'.\n\nThis behaviour also prompted the need to design a new test script to\nhave a testing of some niche cases such as those stated before, but\nthis is something that will be covered in the patch series responsible\nfor porting the 'summary' subcommand to C.\n\nComments and reviews are appreciated.\n\nThanks,\nShourya Shukla\n\nShourya Shukla (4):\n t7401: modernize style\n t7401: change test_i18ncmp syntax for clarity\n t7401: ensure uniformity in the '--for-status' test\n t7401: add a WARNING and a NEEDSWORK\n\n t/t7401-submodule-summary.sh | 26 +++++++++++++++++---------\n 1 file changed, 17 insertions(+), 9 deletions(-)\n\n-- \n2.27.0\n\n"},{"id":"402052","messageId":"20200726142553.42545-2-shouryashukla.oo@gmail.com","threadId":"53922","inReplyTo":"20200726142553.42545-1-shouryashukla.oo@gmail.com","subject":"[PATCH 1/4] t7401: modernize style","fromName":"Shourya Shukla","fromEmail":"shouryashukla.oo@gmail.com","sentAt":"2020-07-26T14:25:50Z","receivedAt":"2020-07-26T14:26:16Z","isPatch":true,"sender":{"key":"shouryashukla.oo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43680618?v=4"},"body":"The tests in 't7401-submodule-summary.sh' were written a long time ago\nand have some violations with respect to our CodingGuidelines such as\nincorrect spacing in usages of the redirection operator and absence\nof line feed between statements in case of the '|' (pipe) operator.\nUpdate it to match the CodingGuidelines.\n\nMentored-by: Christian Couder \nMentored-by: Kaartic Sivaraam \nSigned-off-by: Shourya Shukla \n---\n t/t7401-submodule-summary.sh | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 9bc841d085..4439fb7c17 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -16,12 +16,13 @@ add_file () {\n \towd=$(pwd)\n \tcd \"$sm\"\n \tfor name; do\n-\t\techo \"$name\" > \"$name\" &&\n+\t\techo \"$name\" >\"$name\" &&\n \t\tgit add \"$name\" &&\n \t\ttest_tick &&\n \t\tgit commit -m \"Add $name\"\n \tdone >/dev/null\n-\tgit rev-parse --verify HEAD | cut -c1-7\n+\tgit rev-parse --verify HEAD |\n+\tcut -c1-7\n \tcd \"$owd\"\n }\n commit_file () {\n@@ -125,7 +126,8 @@ commit_file sm1 &&\n head3=$(\n \tcd sm1 &&\n \tgit reset --hard HEAD~2 >/dev/null &&\n-\tgit rev-parse --verify HEAD | cut -c1-7\n+\tgit rev-parse --verify HEAD |\n+\tcut -c1-7\n )\n \n test_expect_success 'modified submodule(backward)' \"\n-- \n2.27.0\n\n"},{"id":"402053","messageId":"20200726142553.42545-3-shouryashukla.oo@gmail.com","threadId":"53922","inReplyTo":"20200726142553.42545-1-shouryashukla.oo@gmail.com","subject":"[PATCH 2/4] t7401: change test_i18ncmp syntax for clarity","fromName":"Shourya Shukla","fromEmail":"shouryashukla.oo@gmail.com","sentAt":"2020-07-26T14:25:51Z","receivedAt":"2020-07-26T14:26:19Z","isPatch":true,"sender":{"key":"shouryashukla.oo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43680618?v=4"},"body":"Change the test_i18ncmp syntax from 'test_i18ncmp actual expected' to\n'test_i18ncmp expected actual' to align it with the convention followed\nby other tests in the test script.\n\nMentored-by: Christian Couder \nMentored-by: Kaartic Sivaraam \nSigned-off-by: Shourya Shukla \n---\n t/t7401-submodule-summary.sh | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 4439fb7c17..18fefdb0ba 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -183,7 +183,7 @@ test_expect_success 'typechanged submodule(submodule->blob), --cached' \"\n < Add foo5\n \n EOF\n-\ttest_i18ncmp actual expected\n+\ttest_i18ncmp expected actual\n \"\n \n test_expect_success 'typechanged submodule(submodule->blob), --files' \"\n@@ -193,7 +193,7 @@ test_expect_success 'typechanged submodule(submodule->blob), --files' \"\n > Add foo5\n \n EOF\n-\ttest_i18ncmp actual expected\n+\ttest_i18ncmp expected actual\n \"\n \n rm -rf sm1 &&\n@@ -204,7 +204,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' \"\n * sm1 $head4(submodule)->$head5(blob):\n \n EOF\n-\ttest_i18ncmp actual expected\n+\ttest_i18ncmp expected actual\n \"\n \n rm -f sm1 &&\n@@ -217,7 +217,7 @@ test_expect_success 'nonexistent commit' \"\n Warn: sm1 doesn't contain commit $head4_full\n \n EOF\n-\ttest_i18ncmp actual expected\n+\ttest_i18ncmp expected actual\n \"\n \n commit_file\n-- \n2.27.0\n\n"},{"id":"402054","messageId":"20200726142553.42545-4-shouryashukla.oo@gmail.com","threadId":"53922","inReplyTo":"20200726142553.42545-1-shouryashukla.oo@gmail.com","subject":"[PATCH 3/4] t7401: ensure uniformity in the '--for-status' test","fromName":"Shourya Shukla","fromEmail":"shouryashukla.oo@gmail.com","sentAt":"2020-07-26T14:25:52Z","receivedAt":"2020-07-26T14:26:22Z","isPatch":true,"sender":{"key":"shouryashukla.oo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43680618?v=4"},"body":"The '--for-status' test got its expected output from stdin. This is\ninconsistent with the other tests in the test script which get their\nexpected output from a file named 'expected'.\n\nSo, change the syntax of the '--for-status' test for uniformity.\n\nMentored-by: Christian Couder \nMentored-by: Kaartic Sivaraam \nSigned-off-by: Shourya Shukla \n---\n t/t7401-submodule-summary.sh | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 18fefdb0ba..145914cd5a 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -285,13 +285,14 @@ EOF\n \n test_expect_success '--for-status' \"\n \tgit submodule summary --for-status HEAD^ >actual &&\n-\ttest_i18ncmp actual - <expected <<-EOF &&\n * sm1 $head6...0000000:\n \n * sm2 0000000...$head7 (2):\n > Add foo9\n \n EOF\n+\ttest_i18ncmp expected actual\n \"\n \n test_expect_success 'fail when using --files together with --cached' \"\n-- \n2.27.0\n\n"},{"id":"402055","messageId":"20200726142553.42545-5-shouryashukla.oo@gmail.com","threadId":"53922","inReplyTo":"20200726142553.42545-1-shouryashukla.oo@gmail.com","subject":"[PATCH 4/4] t7401: add a WARNING and a NEEDSWORK","fromName":"Shourya Shukla","fromEmail":"shouryashukla.oo@gmail.com","sentAt":"2020-07-26T14:25:53Z","receivedAt":"2020-07-26T14:26:26Z","isPatch":true,"sender":{"key":"shouryashukla.oo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43680618?v=4"},"body":"Add a WARNING regarding the usage of 'git add' instead of 'git\nsubmodule add' to add submodules to the superproject. Also add a\nNEEDSWORK regarding the outdated syntax and working of the test, which\nmay need to be improved to obtain better and desired results.\n\nWhile at it, change the word 'test' to 'test script' in the test\ndescription to avoid ambiguity.\n\nMentored-by: Christian Couder \nMentored-by: Kaartic Sivaraam \nSigned-off-by: Shourya Shukla \n---\n t/t7401-submodule-summary.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex 145914cd5a..2db4cf5cbf 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -5,8 +5,13 @@\n \n test_description='Summary support for submodules\n \n-This test tries to verify the sanity of summary subcommand of git submodule.\n+This test script tries to verify the sanity of summary subcommand of git submodule.\n '\n+# WARNING: This test script uses 'git add' instead of 'git submodule add' to add\n+# submodules to the superproject. Some submodule subcommands such as init and\n+# deinit might not work as expected in this script.\n+\n+# NEEDSWORK: This test script is old fashioned and may need a big cleanup.\n \n . ./test-lib.sh\n \n-- \n2.27.0\n\n"}]}