On 20/02/18 17:19, Junio C Hamano wrote:
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
test_expect_success 'setup fake editor' '
- echo "#!$SHELL_PATH" >fake_editor.sh &&
- cat >>fake_editor.sh <<-\EOF &&
+ FAKE_EDITOR="$(pwd)/fake-editor.sh" &&
+ write_script "$FAKE_EDITOR" <<-\EOF &&
mv -f "$1" oldpatch &&
mv -f patch "$1"
EOF
- chmod a+x fake_editor.sh &&
- test_set_editor "$(pwd)/fake_editor.sh"
+ test_set_editor "$FAKE_EDITOR"
'
The very first thing that test_set_editor() does is set FAKE_EDITOR to
the value of $1, so it is confusing to see it getting setting it here
first; the reader has to spend extra brain cycles wondering if
something non-obvious is going on that requires this manual
assignment. Perhaps drop the assignment altogether and just write
literal "fake_editor.sh" in the couple places it's needed (as was done
in the original code)?
Yeah, I think $(pwd)/ prefix is needed at the final step (i.e. as
the first argument to test_set_editor) for this to be a faithful
rewrite but it is distracting having to write it anywhere else.
Other than that, this looks like a quite straight-forward cleanup.
Thanks, both. Here is what I'd be queuing tentatively.
That looks good, thanks for fixing it up
Phillip
t/t3701-add-interactive.sh | 33 +++++----------------------------
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 39c7423069..4a369fcb51 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -87,13 +87,8 @@ test_expect_success 'setup expected' '
EOF
'
-test_expect_success 'setup fake editor' '
- >fake_editor.sh &&
- chmod a+x fake_editor.sh &&
- test_set_editor "$(pwd)/fake_editor.sh"
-'
-
test_expect_success 'dummy edit works' '
+ test_set_editor : &&
(echo e; echo a) | git add -p &&
git diff > diff &&
test_cmp expected diff
@@ -110,12 +105,10 @@ test_expect_success 'setup patch' '
'
test_expect_success 'setup fake editor' '
- echo "#!$SHELL_PATH" >fake_editor.sh &&
- cat >>fake_editor.sh <<-\EOF &&
+ write_script "fake_editor.sh" <<-\EOF &&
mv -f "$1" oldpatch &&
mv -f patch "$1"
EOF
- chmod a+x fake_editor.sh &&
test_set_editor "$(pwd)/fake_editor.sh"
'
@@ -302,18 +295,12 @@ test_expect_success 'deleting an empty file' '
test_expect_success 'split hunk setup' '
git reset --hard &&
- for i in 10 20 30 40 50 60
- do
- echo $i
- done >test &&
+ test_write_lines 10 20 30 40 50 60 >test &&
git add test &&
test_tick &&
git commit -m test &&
- for i in 10 15 20 21 22 23 24 30 40 50 60
- do
- echo $i
- done >test
+ test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test
'
test_expect_success 'split hunk "add -p (edit)"' '
@@ -334,17 +321,7 @@ test_expect_success 'split hunk "add -p (edit)"' '
'
test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
- cat >test <<-\EOF &&
- 5
- 10
- 20
- 21
- 30
- 31
- 40
- 50
- 60
- EOF
+ test_write_lines 5 10 20 21 30 31 40 50 60 >test &&
git reset &&
# test sequence is s(plit), n(o), y(es), e(dit)
# q n q q is there to make sure we exit at the end.
