Re: [PATCH 1/3] t3404: restore specialized rebase-editor following commentchar test

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 12, 2013, 07:29 UTC
Message-ID: <CAPig+cQpNTFr91wp8DxYs3tabmj4xdmqTQ4_ZoNhp-4+0BUKCA@mail.gmail.com>
In-Reply-To: <7v4nav1kzx.fsf@alter.siamese.dyndns.org>

On Mon, Aug 12, 2013 at 2:28 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> At start of script, t3404 installs a specialized test-editor ($EDITOR)
>> upon which many of the interactive rebase tests depend.  Late in t3404,
>> test "rebase -i respects core.commentchar" installs its own custom
>> editor but neglects to restore the specialized editor when finished.
>> This oversight will cause later tests, which require the specialized
>> editor, to fail.
>
> That is not oversight but was deliberately done knowing that it will
> be the last test (and new tests can be added before it).

There is no mention of this being deliberate either in the mailing list discussion [1] or the commit message (180bad3d1), and other tests have been added following this one.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/216079

Show 5 quoted lines

> I think the patch is one way to give _known_ status to later tests
> by declaring the editor installed by "set_fake_editor" the gold
> standard, but isn't a better alternative to make sure that any newly
> added tests after this point (or before the commentchar tests, for
> that matter) set a fake editor it wants to use explicitly?

set_fake_editor is the very first thing done by t3404, and many tests depend upon this state. It would have been inconsistent for this one new test to be the exception by having to invoke set_fake_editor itself, however, I don't mind making the new test more self-contained, despite the inconsistency. (A later cleanup patch can do the same for other existing tests.)