Re: [PATCH 1/3] t3401: add another directory rename testcase for rebase and am

[Elijah Newren <newren@gmail.com>]

On Thu, Aug 30, 2018 at 9:01 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@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, looks great.


> 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 &&