Re: [PATCH v6 7/7] git: submodule honor -c credential.* from command line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> On Mon, Feb 29, 2016 at 2:58 PM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
>>
>> +test_expect_success 'cmdline credential config passes into submodules' '
>> +       git init super &&
>> +       set_askpass user@host pass@host &&
>
> I wonder why we use pass@host as a password, it seems confusing to me.
> p@ssword would have worked if we wanted to test a password containing an @,
> pass@host doesn't quite fit my mental model of how passwords work.
> No need to change anything, better be consistent with the rest of the tests.
>
>
>> +       (
>> +               cd super &&
>> +               git submodule add "$HTTPD_URL/auth/dumb/repo.git" sub &&
>> +               git commit -m "add submodule"
>> +       ) &&
>> +       set_askpass wrong pass@host &&
>> +       test_must_fail git clone --recursive super super-clone &&
>> +       rm -rf super-clone &&
>> +       set_askpass wrong pass@host &&
>
> Why set set_askpass a second time here?

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.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]