From 94e06c905740eaec7501fc026598b5839cc63fba Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Fri, 21 Aug 2020 22:29:48 +0530 Subject: [PATCH 1/5] t7401: modernize style The tests in 't7401-submodule-summary.sh' were written a long time ago and has a violation with respect to our CodingGuidelines which is, incorrect spacing in usages of the redirection operator. Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Helped-by: Denton Liu Helped-by: Taylor Blau Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- t/t7401-submodule-summary.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 9bc841d085..07d4ba0b26 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -16,7 +16,7 @@ add_file () { owd=$(pwd) cd "$sm" for name; do - echo "$name" > "$name" && + echo "$name" >"$name" && git add "$name" && test_tick && git commit -m "Add $name" From 7303da30021761b3a7c862d795e67ab74b5d66eb Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Fri, 21 Aug 2020 22:29:49 +0530 Subject: [PATCH 2/5] t7401: use 'short' instead of 'verify' and cut in rev-parse calls 'git rev-parse' can limit the number of characters in the hash it outputs using the '--short' option, thereby, making the 'cut' invocation redundant. Since using '--short' implies '--verify' as well, we can safely replace the latter with the former. This change results in the helper functions getting the hash in the same way 'summary' gets the hash internally. So, avoid the unnecessary invocation to 'cut' in the helper functions. Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- t/t7401-submodule-summary.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 07d4ba0b26..ccbac875fe 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -21,7 +21,7 @@ add_file () { test_tick && git commit -m "Add $name" done >/dev/null - git rev-parse --verify HEAD | cut -c1-7 + git rev-parse --short HEAD cd "$owd" } commit_file () { @@ -125,7 +125,7 @@ commit_file sm1 && head3=$( cd sm1 && git reset --hard HEAD~2 >/dev/null && - git rev-parse --verify HEAD | cut -c1-7 + git rev-parse --short HEAD ) test_expect_success 'modified submodule(backward)' " From 17c102e30dc3a7c9306112217cabd99365ae4fb8 Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Fri, 21 Aug 2020 22:29:50 +0530 Subject: [PATCH 3/5] t7401: change syntax of test_i18ncmp calls for clarity Change the test_i18ncmp syntax from 'test_i18ncmp actual expected' to 'test_i18ncmp expected actual' to align it with the convention followed by other tests in the test script. Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- t/t7401-submodule-summary.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index ccbac875fe..3f580455f7 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -181,7 +181,7 @@ test_expect_success 'typechanged submodule(submodule->blob), --cached' " < Add foo5 EOF - test_i18ncmp actual expected + test_i18ncmp expected actual " test_expect_success 'typechanged submodule(submodule->blob), --files' " @@ -191,7 +191,7 @@ test_expect_success 'typechanged submodule(submodule->blob), --files' " > Add foo5 EOF - test_i18ncmp actual expected + test_i18ncmp expected actual " rm -rf sm1 && @@ -202,7 +202,7 @@ test_expect_success 'typechanged submodule(submodule->blob)' " * sm1 $head4(submodule)->$head5(blob): EOF - test_i18ncmp actual expected + test_i18ncmp expected actual " rm -f sm1 && @@ -215,7 +215,7 @@ test_expect_success 'nonexistent commit' " Warn: sm1 doesn't contain commit $head4_full EOF - test_i18ncmp actual expected + test_i18ncmp expected actual " commit_file @@ -283,7 +283,7 @@ EOF test_expect_success '--for-status' " git submodule summary --for-status HEAD^ >actual && - test_i18ncmp actual - < Date: Fri, 21 Aug 2020 22:29:51 +0530 Subject: [PATCH 4/5] t7401: change indentation for enhanced readability Change the indentation of expected outputs for enhanced readability of the tests. Also modify the heredoc string limiter in a test which lacks it to support the indentation change. Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Helped-by: Junio C Hamano Helped-by: Taylor Blau Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- t/t7401-submodule-summary.sh | 130 +++++++++++++++++------------------ 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 3f580455f7..1491ab6448 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -38,10 +38,10 @@ test_expect_success 'added submodule' " git add sm1 && git submodule summary >actual && cat >expected <<-EOF && -* sm1 0000000...$head1 (2): - > Add foo2 + * sm1 0000000...$head1 (2): + > Add foo2 -EOF + EOF test_cmp expected actual " @@ -52,10 +52,10 @@ test_expect_success 'added submodule (subdirectory)' " git submodule summary >../actual ) && cat >expected <<-EOF && -* ../sm1 0000000...$head1 (2): - > Add foo2 + * ../sm1 0000000...$head1 (2): + > Add foo2 -EOF + EOF test_cmp expected actual " @@ -73,10 +73,10 @@ test_expect_success 'added submodule (subdirectory with explicit path)' " git submodule summary ../sm1 >../actual ) && cat >expected <<-EOF && -* ../sm1 0000000...$head1 (2): - > Add foo2 + * ../sm1 0000000...$head1 (2): + > Add foo2 -EOF + EOF test_cmp expected actual " @@ -86,20 +86,20 @@ head2=$(add_file sm1 foo3) test_expect_success 'modified submodule(forward)' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head1...$head2 (1): - > Add foo3 + * sm1 $head1...$head2 (1): + > Add foo3 -EOF + EOF test_cmp expected actual " test_expect_success 'modified submodule(forward), --files' " git submodule summary --files >actual && cat >expected <<-EOF && -* sm1 $head1...$head2 (1): - > Add foo3 + * sm1 $head1...$head2 (1): + > Add foo3 -EOF + EOF test_cmp expected actual " @@ -110,10 +110,10 @@ test_expect_success 'no ignore=all setting has any effect' " git config diff.ignoreSubmodules all && git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head1...$head2 (1): - > Add foo3 + * sm1 $head1...$head2 (1): + > Add foo3 -EOF + EOF test_cmp expected actual && git config --unset diff.ignoreSubmodules && git config --remove-section submodule.sm1 && @@ -131,11 +131,11 @@ head3=$( test_expect_success 'modified submodule(backward)' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head2...$head3 (2): - < Add foo3 - < Add foo2 + * sm1 $head2...$head3 (2): + < Add foo3 + < Add foo2 -EOF + EOF test_cmp expected actual " @@ -144,25 +144,25 @@ head4_full=$(GIT_DIR=sm1/.git git rev-parse --verify HEAD) test_expect_success 'modified submodule(backward and forward)' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head2...$head4 (4): - > Add foo5 - > Add foo4 - < Add foo3 - < Add foo2 + * sm1 $head2...$head4 (4): + > Add foo5 + > Add foo4 + < Add foo3 + < Add foo2 -EOF + EOF test_cmp expected actual " test_expect_success '--summary-limit' " git submodule summary -n 3 >actual && cat >expected <<-EOF && -* sm1 $head2...$head4 (4): - > Add foo5 - > Add foo4 - < Add foo3 + * sm1 $head2...$head4 (4): + > Add foo5 + > Add foo4 + < Add foo3 -EOF + EOF test_cmp expected actual " @@ -177,20 +177,20 @@ mv sm1-bak sm1 test_expect_success 'typechanged submodule(submodule->blob), --cached' " git submodule summary --cached >actual && cat >expected <<-EOF && -* sm1 $head4(submodule)->$head5(blob) (3): - < Add foo5 + * sm1 $head4(submodule)->$head5(blob) (3): + < Add foo5 -EOF + EOF test_i18ncmp expected actual " test_expect_success 'typechanged submodule(submodule->blob), --files' " git submodule summary --files >actual && cat >expected <<-EOF && -* sm1 $head5(blob)->$head4(submodule) (3): - > Add foo5 + * sm1 $head5(blob)->$head4(submodule) (3): + > Add foo5 -EOF + EOF test_i18ncmp expected actual " @@ -199,9 +199,9 @@ git checkout-index sm1 test_expect_success 'typechanged submodule(submodule->blob)' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head4(submodule)->$head5(blob): + * sm1 $head4(submodule)->$head5(blob): -EOF + EOF test_i18ncmp expected actual " @@ -211,10 +211,10 @@ head6=$(add_file sm1 foo6 foo7) test_expect_success 'nonexistent commit' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head4...$head6: - Warn: sm1 doesn't contain commit $head4_full + * sm1 $head4...$head6: + Warn: sm1 doesn't contain commit $head4_full -EOF + EOF test_i18ncmp expected actual " @@ -222,10 +222,10 @@ commit_file test_expect_success 'typechanged submodule(blob->submodule)' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head5(blob)->$head6(submodule) (2): - > Add foo7 + * sm1 $head5(blob)->$head6(submodule) (2): + > Add foo7 -EOF + EOF test_i18ncmp expected actual " @@ -234,9 +234,9 @@ rm -rf sm1 test_expect_success 'deleted submodule' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head6...0000000: + * sm1 $head6...0000000: -EOF + EOF test_cmp expected actual " @@ -249,22 +249,22 @@ test_expect_success 'create second submodule' ' test_expect_success 'multiple submodules' " git submodule summary >actual && cat >expected <<-EOF && -* sm1 $head6...0000000: + * sm1 $head6...0000000: -* sm2 0000000...$head7 (2): - > Add foo9 + * sm2 0000000...$head7 (2): + > Add foo9 -EOF + EOF test_cmp expected actual " test_expect_success 'path filter' " git submodule summary sm2 >actual && cat >expected <<-EOF && -* sm2 0000000...$head7 (2): - > Add foo9 + * sm2 0000000...$head7 (2): + > Add foo9 -EOF + EOF test_cmp expected actual " @@ -272,24 +272,24 @@ commit_file sm2 test_expect_success 'given commit' " git submodule summary HEAD^ >actual && cat >expected <<-EOF && -* sm1 $head6...0000000: + * sm1 $head6...0000000: -* sm2 0000000...$head7 (2): - > Add foo9 + * sm2 0000000...$head7 (2): + > Add foo9 -EOF + EOF test_cmp expected actual " test_expect_success '--for-status' " git submodule summary --for-status HEAD^ >actual && - test_i18ncmp - actual < Add foo9 + * sm2 0000000...$head7 (2): + > Add foo9 -EOF + EOF " test_expect_success 'fail when using --files together with --cached' " From 2a0d1a5ce2a945dcc590537f69c36c152cce4042 Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Fri, 21 Aug 2020 22:29:52 +0530 Subject: [PATCH 5/5] t7401: add a NEEDSWORK Add a NEEDSWORK regarding the outdated syntax and working of the test, which may need to be improved to obtain better and desired results. While at it, change the word 'test' to 'test script' in the test description to avoid ambiguity. Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Helped-by: Taylor Blau Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- t/t7401-submodule-summary.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 1491ab6448..cc87d26619 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -5,8 +5,11 @@ test_description='Summary support for submodules -This test tries to verify the sanity of summary subcommand of git submodule. +This test script tries to verify the sanity of summary subcommand of git submodule. ' +# NEEDSWORK: This test script is old fashioned and may need a big cleanup due to +# various reasons, one of them being that there are lots of commands taking place +# outside of 'test_expect_success' block, which is no longer in good-style. . ./test-lib.sh