| Message ID | 20201021124823.2217-4-charvi077@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] t7201: put each command on a separate line | expand |
On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > According to Documentation/CodingGuidelines, redirect > operator is written with space before, but no space > after them. > > Let's remove these whitespaces after redirect operators. > > Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> > --- > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh > @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' ' > -cat > expect << EOF > +cat >expect << EOF There's still a space after the '<<' operator which should be removed: cat >expect <<EOF
Eric Sunshine <sunshine@sunshineco.com> writes: > On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote: >> According to Documentation/CodingGuidelines, redirect >> operator is written with space before, but no space >> after them. >> >> Let's remove these whitespaces after redirect operators. >> >> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> >> --- >> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh >> @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' ' >> -cat > expect << EOF >> +cat >expect << EOF > > There's still a space after the '<<' operator which should be removed: > > cat >expect <<EOF Also, this is outside any test_expect_*, which is unusual in modernized test scripts. There are many other instances of preparing expected output outside test_expect_* in this file, so we may need another patch to clean them up. For now, within the context of this patch, let's just fix the space after the << here-doc redirection operator, as you spotted. The attached I'll squash into this patch. Thanks. t/t7102-reset.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git i/t/t7102-reset.sh w/t/t7102-reset.sh index a8c96bf162..07acaa2beb 100755 --- i/t/t7102-reset.sh +++ w/t/t7102-reset.sh @@ -460,7 +460,7 @@ test_expect_success 'resetting an unmodified path is a no-op' ' git diff-index --cached --exit-code HEAD ' -cat >expect << EOF +cat >expect <<EOF Unstaged changes after reset: M file2 EOF
On Thu, Oct 22, 2020 at 1:44 AM Junio C Hamano <gitster@pobox.com> wrote: > Eric Sunshine <sunshine@sunshineco.com> writes: > > There's still a space after the '<<' operator which should be removed: > > > > cat >expect <<EOF > > Also, this is outside any test_expect_*, which is unusual in > modernized test scripts. There are many other instances of > preparing expected output outside test_expect_* in this file, > so we may need another patch to clean them up. I noticed that, as well, but wasn't sure if mentioning it was worthwhile considering that this is a microproject and we're already at v5. It can certainly be addressed by someone as a follow-on patch if desired. > For now, within the context of this patch, let's just fix the space > after the << here-doc redirection operator, as you spotted. The > attached I'll squash into this patch. > > -cat >expect << EOF > +cat >expect <<EOF Looks fine.
Junio C Hamano <gitster@pobox.com> writes: > Also, this is outside any test_expect_*, which is unusual in > modernized test scripts. There are many other instances of > preparing expected output outside test_expect_* in this file, > so we may need another patch to clean them up. > > For now, within the context of this patch, let's just fix the space > after the << here-doc redirection operator, as you spotted. The > attached I'll squash into this patch. And the other clean-up patch would look like this. --- >8 --- Subject: t7102: prepare expected output inside test_expect_* block That way we can notice if there is a breakage/bug in the parts of the test that prepare the expected outcome, which is how modern tests are arranged. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t7102-reset.sh | 244 +++++++++++++++++++++++++++---------------------------- 1 file changed, 121 insertions(+), 123 deletions(-) diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh index 07acaa2beb..821e8bb94d 100755 --- c/t/t7102-reset.sh +++ w/t/t7102-reset.sh @@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' ' test_i18ncmp .expected .actual ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF - test_expect_success 'giving a non existing revision should fail' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF + test_must_fail git reset aaaaaa && test_must_fail git reset --mixed aaaaaa && test_must_fail git reset --soft aaaaaa && @@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not check_changes $head5 ' ->.diff_expect -cat >.cached_expect <<EOF -diff --git a/secondfile b/secondfile -index $head5p1s..$head5s 100644 ---- a/secondfile -+++ b/secondfile -@@ -1 +1,2 @@ --2nd file -+1st line 2nd file -+2nd line 2nd file -EOF -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF test_expect_success '--soft reset only should show changes in diff --cached' ' + >.diff_expect && + cat >.cached_expect <<-EOF && + diff --git a/secondfile b/secondfile + index $head5p1s..$head5s 100644 + --- a/secondfile + +++ b/secondfile + @@ -1 +1,2 @@ + -2nd file + +1st line 2nd file + +2nd line 2nd file + EOF + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF git reset --soft HEAD^ && check_changes $head5p1 && test "$(git rev-parse ORIG_HEAD)" = \ $head5 ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -3rd line 2nd file -EOF test_expect_success 'changing files and redo the last commit should succeed' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + 3rd line 2nd file + EOF echo "3rd line 2nd file" >>secondfile && git commit -a -C ORIG_HEAD && head4=$(git rev-parse --verify HEAD) && @@ -231,54 +231,54 @@ test_expect_success 'changing files and redo the last commit should succeed' ' $head5 ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -first: -1st file -2nd line 1st file -second: -2nd file -EOF test_expect_success '--hard reset should change the files and undo commits permanently' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + first: + 1st file + 2nd line 1st file + second: + 2nd file + EOF git reset --hard HEAD~2 && check_changes $head5p2 && test "$(git rev-parse ORIG_HEAD)" = \ $head4 ' ->.diff_expect -cat >.cached_expect <<EOF -diff --git a/first b/first -deleted file mode 100644 -index $head5p2f..0000000 ---- a/first -+++ /dev/null -@@ -1,2 +0,0 @@ --1st file --2nd line 1st file -diff --git a/second b/second -deleted file mode 100644 -index $head5p1s..0000000 ---- a/second -+++ /dev/null -@@ -1 +0,0 @@ --2nd file -diff --git a/secondfile b/secondfile -new file mode 100644 -index 0000000..$head5s ---- /dev/null -+++ b/secondfile -@@ -0,0 +1,2 @@ -+1st line 2nd file -+2nd line 2nd file -EOF -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF test_expect_success 'redoing changes adding them without commit them should succeed' ' + >.diff_expect && + cat >.cached_expect <<-EOF && + diff --git a/first b/first + deleted file mode 100644 + index $head5p2f..0000000 + --- a/first + +++ /dev/null + @@ -1,2 +0,0 @@ + -1st file + -2nd line 1st file + diff --git a/second b/second + deleted file mode 100644 + index $head5p1s..0000000 + --- a/second + +++ /dev/null + @@ -1 +0,0 @@ + -2nd file + diff --git a/secondfile b/secondfile + new file mode 100644 + index 0000000..$head5s + --- /dev/null + +++ b/secondfile + @@ -0,0 +1,2 @@ + +1st line 2nd file + +2nd line 2nd file + EOF + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF git rm first && git mv second secondfile && @@ -288,46 +288,45 @@ test_expect_success 'redoing changes adding them without commit them should succ check_changes $head5p2 ' -cat >.diff_expect <<EOF -diff --git a/first b/first -deleted file mode 100644 -index $head5p2f..0000000 ---- a/first -+++ /dev/null -@@ -1,2 +0,0 @@ --1st file --2nd line 1st file -diff --git a/second b/second -deleted file mode 100644 -index $head5p1s..0000000 ---- a/second -+++ /dev/null -@@ -1 +0,0 @@ --2nd file -EOF ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF test_expect_success '--mixed reset to HEAD should unadd the files' ' + cat >.diff_expect <<-EOF && + diff --git a/first b/first + deleted file mode 100644 + index $head5p2f..0000000 + --- a/first + +++ /dev/null + @@ -1,2 +0,0 @@ + -1st file + -2nd line 1st file + diff --git a/second b/second + deleted file mode 100644 + index $head5p1s..0000000 + --- a/second + +++ /dev/null + @@ -1 +0,0 @@ + -2nd file + EOF + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF git reset && check_changes $head5p2 && test "$(git rev-parse ORIG_HEAD)" = $head5p2 ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF test_expect_success 'redoing the last two commits should succeed' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF git add secondfile && git reset --hard $head5p2 && - git rm first && git mv second secondfile && git commit -a -m "remove 1st and rename 2nd" && @@ -340,15 +339,15 @@ test_expect_success 'redoing the last two commits should succeed' ' check_changes $head5 ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -3rd line in branch2 -EOF test_expect_success '--hard reset to HEAD should clear a failed merge' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + 3rd line in branch2 + EOF git branch branch1 && git branch branch2 && @@ -366,14 +365,14 @@ test_expect_success '--hard reset to HEAD should clear a failed merge' ' check_changes $head3 ' ->.diff_expect ->.cached_expect -cat >.cat_expect <<EOF -secondfile: -1st line 2nd file -2nd line 2nd file -EOF test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge' ' + >.diff_expect && + >.cached_expect && + cat >.cat_expect <<-\EOF && + secondfile: + 1st line 2nd file + 2nd line 2nd file + EOF git reset --hard HEAD^ && check_changes $head5 && @@ -460,12 +459,11 @@ test_expect_success 'resetting an unmodified path is a no-op' ' git diff-index --cached --exit-code HEAD ' -cat >expect <<EOF -Unstaged changes after reset: -M file2 -EOF - test_expect_success '--mixed refreshes the index' ' + cat >expect <<-\EOF && + Unstaged changes after reset: + M file2 + EOF echo 123 >> file2 && git reset --mixed HEAD >output && test_i18ncmp expect output
On Thu, Oct 22, 2020 at 1:56 AM Junio C Hamano <gitster@pobox.com> wrote: > Subject: t7102: prepare expected output inside test_expect_* block > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh > @@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' ' > ->.diff_expect > ->.cached_expect > -cat >.cat_expect <<EOF > -secondfile: > -EOF > - > test_expect_success 'giving a non existing revision should fail' ' > + >.diff_expect && > + >.cached_expect && > + cat >.cat_expect <<-\EOF && > + secondfile: > + EOF You used <<-\EOF rather than plain <<-EOF when possible. Good. (Might be worth mention in the commit message, but perhaps too minor?) > @@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not > test_expect_success '--soft reset only should show changes in diff --cached' ' > + >.diff_expect && > + cat >.cached_expect <<-EOF && > + diff --git a/secondfile b/secondfile > + index $head5p1s..$head5s 100644 And used plain <<-EOF when necessary. Fine.
On Thu, 22 Oct 2020 at 11:14, Junio C Hamano <gitster@pobox.com> wrote: > > Eric Sunshine <sunshine@sunshineco.com> writes: > > > On Wed, Oct 21, 2020 at 8:51 AM Charvi Mendiratta <charvi077@gmail.com> wrote: > >> According to Documentation/CodingGuidelines, redirect > >> operator is written with space before, but no space > >> after them. > >> > >> Let's remove these whitespaces after redirect operators. > >> > >> Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> > >> --- > >> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh > >> @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' ' > >> -cat > expect << EOF > >> +cat >expect << EOF > > > > There's still a space after the '<<' operator which should be removed: > > > > cat >expect <<EOF > > Also, this is outside any test_expect_*, which is unusual in > modernized test scripts. There are many other instances of > preparing expected output outside test_expect_* in this file, > so we may need another patch to clean them up. > > For now, within the context of this patch, let's just fix the space > after the << here-doc redirection operator, as you spotted. The > attached I'll squash into this patch. > > Thanks. > Thanks Eric and Junio, I am resending the patch with the fix of space after << as mentioned. > t/t7102-reset.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git i/t/t7102-reset.sh w/t/t7102-reset.sh > index a8c96bf162..07acaa2beb 100755 > --- i/t/t7102-reset.sh > +++ w/t/t7102-reset.sh > @@ -460,7 +460,7 @@ test_expect_success 'resetting an unmodified path is a no-op' ' > git diff-index --cached --exit-code HEAD > ' > > -cat >expect << EOF > +cat >expect <<EOF > Unstaged changes after reset: > M file2 > EOF > > >
Eric Sunshine <sunshine@sunshineco.com> writes: > On Thu, Oct 22, 2020 at 1:56 AM Junio C Hamano <gitster@pobox.com> wrote: >> Subject: t7102: prepare expected output inside test_expect_* block >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> >> --- >> diff --git c/t/t7102-reset.sh w/t/t7102-reset.sh >> @@ -82,15 +82,15 @@ test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' ' >> ->.diff_expect >> ->.cached_expect >> -cat >.cat_expect <<EOF >> -secondfile: >> -EOF >> - >> test_expect_success 'giving a non existing revision should fail' ' >> + >.diff_expect && >> + >.cached_expect && >> + cat >.cat_expect <<-\EOF && >> + secondfile: >> + EOF > > You used <<-\EOF rather than plain <<-EOF when possible. Good. > > (Might be worth mention in the commit message, but perhaps too minor?) > >> @@ -191,38 +191,38 @@ test_expect_success 'resetting to HEAD with no changes should succeed and do not >> test_expect_success '--soft reset only should show changes in diff --cached' ' >> + >.diff_expect && >> + cat >.cached_expect <<-EOF && >> + diff --git a/secondfile b/secondfile >> + index $head5p1s..$head5s 100644 > > And used plain <<-EOF when necessary. Fine. Yup. Let's declare victory with v5 plus this one as [patch 6/5], and move on. We both know very well that through the microproject that it has been demonstrated that Charvi can now comfortably work with us in the review discussions. Thanks, both.
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh index 2b4cfb2c83..a8c96bf162 100755 --- a/t/t7102-reset.sh +++ b/t/t7102-reset.sh @@ -70,15 +70,15 @@ check_changes () { test_expect_success 'reset --hard message' ' hex=$(git log -1 --format="%h") && - git reset --hard > .actual && - echo HEAD is now at $hex $(commit_msg) > .expected && + git reset --hard >.actual && + echo HEAD is now at $hex $(commit_msg) >.expected && test_i18ncmp .expected .actual ' test_expect_success 'reset --hard message (ISO8859-1 logoutputencoding)' ' hex=$(git log -1 --format="%h") && - git -c "i18n.logOutputEncoding=$test_encoding" reset --hard > .actual && - echo HEAD is now at $hex $(commit_msg $test_encoding) > .expected && + git -c "i18n.logOutputEncoding=$test_encoding" reset --hard >.actual && + echo HEAD is now at $hex $(commit_msg $test_encoding) >.expected && test_i18ncmp .expected .actual ' @@ -387,25 +387,25 @@ test_expect_success '--hard reset to ORIG_HEAD should clear a fast-forward merge ' test_expect_success 'test --mixed <paths>' ' - echo 1 > file1 && - echo 2 > file2 && + echo 1 >file1 && + echo 2 >file2 && git add file1 file2 && test_tick && git commit -m files && before1=$(git rev-parse --short HEAD:file1) && before2=$(git rev-parse --short HEAD:file2) && git rm file2 && - echo 3 > file3 && - echo 4 > file4 && - echo 5 > file1 && + echo 3 >file3 && + echo 4 >file4 && + echo 5 >file1 && after1=$(git rev-parse --short $(git hash-object file1)) && after4=$(git rev-parse --short $(git hash-object file4)) && git add file1 file3 file4 && git reset HEAD -- file1 file2 file3 && test_must_fail git diff --quiet && - git diff > output && + git diff >output && - cat > expect <<-EOF && + cat >expect <<-EOF && diff --git a/file1 b/file1 index $before1..$after1 100644 --- a/file1 @@ -423,9 +423,9 @@ test_expect_success 'test --mixed <paths>' ' EOF test_cmp expect output && - git diff --cached > output && + git diff --cached >output && - cat > cached_expect <<-EOF && + cat >cached_expect <<-EOF && diff --git a/file4 b/file4 new file mode 100644 index 0000000..$after4 @@ -460,14 +460,14 @@ test_expect_success 'resetting an unmodified path is a no-op' ' git diff-index --cached --exit-code HEAD ' -cat > expect << EOF +cat >expect << EOF Unstaged changes after reset: M file2 EOF test_expect_success '--mixed refreshes the index' ' echo 123 >> file2 && - git reset --mixed HEAD > output && + git reset --mixed HEAD >output && test_i18ncmp expect output ' diff --git a/t/t7201-co.sh b/t/t7201-co.sh index b527f8009c..74553f991b 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -33,7 +33,7 @@ fill () { test_expect_success setup ' - fill x y z > same && + fill x y z >same && fill 1 2 3 4 5 6 7 8 >one && fill a b c d e >two && git add same one two && @@ -55,7 +55,7 @@ test_expect_success setup ' git checkout -b simple master && rm -f one && - fill a c e > two && + fill a c e >two && git commit -a -m "Simple D one, M two" && git checkout master @@ -95,7 +95,7 @@ test_expect_success 'checkout -m with dirty tree' ' git clean -f && fill 0 1 2 3 4 5 6 7 8 >one && - git checkout -m side > messages && + git checkout -m side >messages && test "$(git symbolic-ref HEAD)" = "refs/heads/side" && @@ -159,7 +159,7 @@ test_expect_success 'checkout -m with merge conflict' ' test_expect_success 'format of merge conflict from checkout -m' ' git checkout -f master && git clean -f && - fill b d > two && + fill b d >two && git checkout -m simple && git ls-files >current && @@ -182,7 +182,7 @@ test_expect_success 'format of merge conflict from checkout -m' ' test_expect_success 'checkout --merge --conflict=diff3 <branch>' ' git checkout -f master && git reset --hard && git clean -f && - fill b d > two && + fill b d >two && git checkout --merge --conflict=diff3 simple && cat <<-EOF >expect &&
According to Documentation/CodingGuidelines, redirect operator is written with space before, but no space after them. Let's remove these whitespaces after redirect operators. Signed-off-by: Charvi Mendiratta <charvi077@gmail.com> --- t/t7102-reset.sh | 30 +++++++++++++++--------------- t/t7201-co.sh | 10 +++++----- 2 files changed, 20 insertions(+), 20 deletions(-)