From: Jacob Keller <jacob.keller@gmail.com>
Date: Tue, 01 Mar 2016 18:05:34 GMT
Subject: Re: [PATCH v6 7/7] git: submodule honor -c credential.* from command line
Message-ID: <CA+P7+xp41mkHjA0CF=69extO4R2Oam2V3sJA7PoqNbHD=9kw+g@mail.gmail.com>
In-Reply-To: <xmqqh9gq85yc.fsf@gitster.mtv.corp.google.com>

On Tue, Mar 1, 2016 at 9:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I find this in t/lib-httpd.sh:
>
>         set_askpass() {
>                 >"$TRASH_DIRECTORY/askpass-query" &&
>                 echo "$1" >"$TRASH_DIRECTORY/askpass-user" &&
>                 echo "$2" >"$TRASH_DIRECTORY/askpass-pass"
>         }
>
> and expect_askpass peeks at askpass-query to see if Git asked the
> right questions.
>
> I think askpass-query is cleared here because the author of the test
> suite expected that the helpers are used in such a way that you
> always (1) set-askpass once, (2) run a Git command that asks
> credentials, (3) use expect_askpass to validate and do these three
> steps as a logical unit?
>
> That "clone" the test expects to fail does ask the credential, so
> even though the test does not check if the "clone" asked the right
> question, it finishes the three-step logical unit, and then you need
> to clear askpass-query.
>
> It may have been cleaner if you had clear_askpass_query helper that
> is called (1) at the beginning of set_askpass instead of this manual
> clearing, (2) at the end of expect_askpass, as the exchange has been
> tested already at that point, and (3) in place of expect_askpass if
> you choose not to call it (e.g. this place, instead of the second
> set_askpass, you would say clear_askpass_query), perhaps, but I do
> know if that is worth it.
>

Probably something worth looking at doing in the future.

I could call expect_askpass here at each time but I don't think it
would be meaningful after a test_must_fail.

Thanks,
Jake