Re: [PATCH v4 4/4] t0302: test credential-store support for XDG_CONFIG_HOME

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

 



On Sat, Mar 21, 2015 at 1:46 AM, Paul Tan <pyokagan@xxxxxxxxx> wrote:
> On Thu, Mar 19, 2015 at 3:26 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Wed, Mar 18, 2015 at 3:04 AM, Paul Tan <pyokagan@xxxxxxxxx> wrote:
>>> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
>>> index f61b40c..5b0a666 100755
>>> --- a/t/t0302-credential-store.sh
>>> +++ b/t/t0302-credential-store.sh
>>> @@ -6,4 +6,115 @@ test_description='credential-store tests'
>>> +test_expect_success 'when xdg credentials file does not exist, only write to ~/.git-credentials; do not create xdg file' '
>>
>> These test descriptions are quite long, often mirroring in prose what
>> the test body already says more concisely. I generally try to keep
>> descriptions short while still being descriptive enough to give a
>> general idea about what is being tested. I've rewritten a few test
>> descriptions (below) to be very short, in fact probably too terse; but
>> perhaps better descriptions would lie somewhere between the two
>> extremes. First example, for this test:
>>
>>     "!xdg: >.git-credentials !xdg"
>>
> I will make the test descriptions shorter. However, I don't think the
> test descriptions need to be so terse such that a separate key is
> required. e.g. I will shorten the above to "when xdg file does not
> exist, it's not created.", or even terser: "when xdg file not exists,
> it's not created.". I don't think symbols should be used, as many
> other test descriptions do not use them.

Your updated test descriptions all sound fine.

>>> +XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME && helper_test store
>>> +unset XDG_CONFIG_HOME
>>
>> It's hard to spot the "helper_test store" at the end of line. I'd
>> place it on a line by itself so that it is easy to see that it is
>> wrapped by the setting and unsetting of the environment variable.
>
> Thanks, will fix. Although now it looks weird that the "export" is the
> only one with a continuation on a single line, so I split all of them
> so that they each have their own line.

An &&-chain outside of a test is not meaningful in this case, so I
meant either this:

    XDG_CONFIG_HOME="$HOME/xdg"
    export XDG_CONFIG_HOME
    helper_test store
    unset XDG_CONFIG_HOME

or, slightly more compact (using && just to combine the assignment and
export on one line):

    XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME
    helper_test store
    unset XDG_CONFIG_HOME
--
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]