| Message ID | 20210224195129.4004-13-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userdiff: refactor + test + doc + misc improvements | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > +# check each individual file > +for i in $(git -C t4018 ls-files) > +do > + test_expect_success "setup hunk header: $i" " > + grep -v '^t4018' \"t4018/$i\" >\"t4018/$i.content\" && > + sed -n -e 's/^t4018 header: //p' <\"t4018/$i\" >\"t4018/$i.header\" && > + cp \"t4018/$i.content\" \"$i\" && > > # add test file to the index > - git add "$i" && > + git add \"$i\" && > # place modified file in the worktree > - sed -e "s/ChangeMe/IWasChanged/" <"t4018/$i.content" >"$i" || return 1 > - done > -' > + sed -e 's/ChangeMe/IWasChanged/' <\"t4018/$i.content\" >\"$i\" > + " Please use '' around the second argument (i.e. test body) of the test_expect_success, and use "" inside it. "$i" that is used in the loop is visible perfectly fine inside the test body when it is eval'ed, and we won't have to count ugly backslashes that way. > > -# check each individual file > -for i in $(git ls-files) > -do > test_expect_success "hunk header: $i" " > git diff -U1 $i >diff && > sed -n -e 's/^.*@@$//p' -e 's/^.*@@ //p' <diff >ctx &&
Am 25.02.21 um 03:52 schrieb Junio C Hamano: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> +# check each individual file >> +for i in $(git -C t4018 ls-files) >> +do >> + test_expect_success "setup hunk header: $i" " >> + grep -v '^t4018' \"t4018/$i\" >\"t4018/$i.content\" && >> + sed -n -e 's/^t4018 header: //p' <\"t4018/$i\" >\"t4018/$i.header\" && >> + cp \"t4018/$i.content\" \"$i\" && >> >> # add test file to the index >> - git add "$i" && >> + git add \"$i\" && >> # place modified file in the worktree >> - sed -e "s/ChangeMe/IWasChanged/" <"t4018/$i.content" >"$i" || return 1 >> - done >> -' >> + sed -e 's/ChangeMe/IWasChanged/' <\"t4018/$i.content\" >\"$i\" >> + " > > Please use '' around the second argument (i.e. test body) of the > test_expect_success, and use "" inside it. "$i" that is used in the > loop is visible perfectly fine inside the test body when it is > eval'ed, and we won't have to count ugly backslashes that way. If we do that, then we better be sure that the implementation of test_expect_success does not clobber $i. Looks like we are OK at this time. -- Hannes
Johannes Sixt <j6t@kdbg.org> writes: > Am 25.02.21 um 03:52 schrieb Junio C Hamano: >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: >> >>> +# check each individual file >>> +for i in $(git -C t4018 ls-files) >>> +do >>> + test_expect_success "setup hunk header: $i" " >>> + grep -v '^t4018' \"t4018/$i\" >\"t4018/$i.content\" && >>> + sed -n -e 's/^t4018 header: //p' <\"t4018/$i\" >\"t4018/$i.header\" && >>> + cp \"t4018/$i.content\" \"$i\" && >>> >>> # add test file to the index >>> - git add "$i" && >>> + git add \"$i\" && >>> # place modified file in the worktree >>> - sed -e "s/ChangeMe/IWasChanged/" <"t4018/$i.content" >"$i" || return 1 >>> - done >>> -' >>> + sed -e 's/ChangeMe/IWasChanged/' <\"t4018/$i.content\" >\"$i\" >>> + " >> >> Please use '' around the second argument (i.e. test body) of the >> test_expect_success, and use "" inside it. "$i" that is used in the >> loop is visible perfectly fine inside the test body when it is >> eval'ed, and we won't have to count ugly backslashes that way. > > If we do that, then we better be sure that the implementation of > test_expect_success does not clobber $i. Looks like we are OK at this time. That's a good point. Often we use more specific variable name than $i to take advantage of the 'eval'-ed nature of the test body, and it may probably be wise to do so here, too.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh index 15dcbe735ca..2365f0e361e 100755 --- a/t/t4018-diff-funcname.sh +++ b/t/t4018-diff-funcname.sh @@ -72,24 +72,23 @@ test_expect_success 'setup hunk header tests' ' cp -R "$TEST_DIRECTORY"/t4018 . && git init t4018 && - git -C t4018 add . && + git -C t4018 add . +' - for i in $(git -C t4018 ls-files) - do - grep -v "^t4018" "t4018/$i" >"t4018/$i.content" && - sed -n -e "s/^t4018 header: //p" <"t4018/$i" >"t4018/$i.header" && - cp "t4018/$i.content" "$i" && +# check each individual file +for i in $(git -C t4018 ls-files) +do + test_expect_success "setup hunk header: $i" " + grep -v '^t4018' \"t4018/$i\" >\"t4018/$i.content\" && + sed -n -e 's/^t4018 header: //p' <\"t4018/$i\" >\"t4018/$i.header\" && + cp \"t4018/$i.content\" \"$i\" && # add test file to the index - git add "$i" && + git add \"$i\" && # place modified file in the worktree - sed -e "s/ChangeMe/IWasChanged/" <"t4018/$i.content" >"$i" || return 1 - done -' + sed -e 's/ChangeMe/IWasChanged/' <\"t4018/$i.content\" >\"$i\" + " -# check each individual file -for i in $(git ls-files) -do test_expect_success "hunk header: $i" " git diff -U1 $i >diff && sed -n -e 's/^.*@@$//p' -e 's/^.*@@ //p' <diff >ctx &&
Change the recently amended setup loop in "setup hunk header tests" to instead set up the test data as we test each individual hunk header test. This means we can get rid of the "|| return 1" case and the previous for-loop, and won't spend time on setting up all the data only to e.g. fail on the 1st test when running under "-i". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t4018-diff-funcname.sh | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)