Elijah Newren <new...@gmail.com> writes:

>> Is this a single-shot environment assignment?  That would have been
>> caught with the test linter.
>
> Ugh, yes.  Sorry.
>
> I was trying to allow backporting to 2.18, so tried to build my series
> on that...but moved forward slightly to en/rebase-consistency in order
> to re-use the same testfile (as mentioned in my cover letter).  But
> that meant my branch was missing a0a630192dca
> ("t/check-non-portable-shell: detect "FOO=bar shell_func"",
> 2018-07-13), so the test-linter couldn't catch this for me.

True, I also only caught this during my integration cycle, not
during the review of the patches.

>> Perhaps squashing this in would be sufficient fix?
>
> Thanks for fixing it up.  That works...although now I've spotted
> another minor issue.  The FAKE_LINES setting is only needed for the
> interactive rebase case and can be removed for the other two.  Oops.
> It doesn't hurt, but might confuse readers of the testcase.

Ah, OK.  So the squashable fix would now become like this, all
fixing the ones from the first patch.

> Would you like me to resend a fixup on top of your fixup, resend the
> whole series with the fixes squashed, or just ignore this final
> (cosmetic) issue since it doesn't affect correctness and I've caused
> you enough extra work already?

No worries.  This is what the maintainer does (when s/he is not
playing other roles like a reviewer or an individual contributor).

I'll squash in the following to the 1/3 patch and queue the topic
with the other two patches.

Thanks.

diff --git a/t/t3401-rebase-and-am-rename.sh b/t/t3401-rebase-and-am-rename.sh
index 94bdfbd69c..e0b5111993 100755
--- a/t/t3401-rebase-and-am-rename.sh
+++ b/t/t3401-rebase-and-am-rename.sh
@@ -141,7 +141,7 @@ test_expect_success 'rebase --interactive: NO directory 
rename' '
                git checkout B^0 &&
 
                set_fake_editor &&
-               FAKE_LINES="1" test_must_fail git rebase --interactive A &&
+               test_must_fail env FAKE_LINES="1" git rebase --interactive A &&
 
                git ls-files -s >out &&
                test_line_count = 6 out &&
@@ -160,7 +160,7 @@ test_expect_success 'rebase (am): NO directory rename' '
                git checkout B^0 &&
 
                set_fake_editor &&
-               FAKE_LINES="1" test_must_fail git rebase A &&
+               test_must_fail git rebase A &&
 
                git ls-files -s >out &&
                test_line_count = 6 out &&
@@ -179,7 +179,7 @@ test_expect_success 'rebase --merge: NO directory rename' '
                git checkout B^0 &&
 
                set_fake_editor &&
-               FAKE_LINES="1" test_must_fail git rebase --merge A &&
+               test_must_fail git rebase --merge A &&
 
                git ls-files -s >out &&
                test_line_count = 6 out &&

Reply via email to