Re: [PATCH 2/2] tests: test credential-store XDG support

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Mar 3, 2015, 22:08 UTC
Message-ID: <vpqegp53f9s.fsf@anie.imag.fr>
In-Reply-To: <1425414299-24000-3-git-send-email-pyokagan@gmail.com>

Paul Tan <pyokagan@gmail.com> writes:

> +# Tests for when $XDG_CONFIG_HOME/git/credentials exists but
> +# ~/.git-credentials does not.

As much as possible, put text in the $1 of test_expect_success instead of comments.

> +rm "$HOME/.git-credentials"
> +mkdir -p "${XDG_CONFIG_HOME:-$HOME/.config}/git"
> +echo '' > "${XDG_CONFIG_HOME:-$HOME/.config}/git/credentials"
> +helper_test store

Don't write code outside test_expect_success (read "Do's, don'ts & things to keep in mind" in t/README).

> +test_expect_success '~/.git-credentials will not be created if XDG git-credentials exist' '
> +	test ! -e "$HOME/.git-credentials"
> +'

test_path_is_missing (see test-lib-functions.sh)

> +echo '' > "$HOME/.git-credentials"
> +echo '' > "${XDG_CONFIG_HOME:-$HOME/.config}/git/credentials"

Not even that outside test_expect_success ;-).

Also, no space after > (Documentation/CodingGuidelines).

Show 10 quoted lines

> +test_expect_success 'Credentials are stored in XDG file if both XDG and HOME files exist' '
> +	check approve store <<-\EOF
> +	protocol=https
> +	host=example.com
> +	username=store-user
> +	password=store-pass
> +	EOF
> +	read contents < "${XDG_CONFIG_HOME:-$HOME/.config}/git/credentials"
> +	test ! -z "$contents"
> +	read contents < "$HOME/.git-credentials"

No space after <.

> +	test -z "$contents"
> +'
> +test_expect_success 'Credentials from XDG file are used if the
> +	credentials exist in both XDG and HOME files' '

Blank line between tests please.

Show 10 quoted lines

> +test_expect_success 'Credentials from both XDG and HOME files meeting the criteria are erased' '
> +	check reject $HELPER <<-\EOF &&
> +	protocol=https
> +	host=example.com
> +	EOF
> +	read contents < "${XDG_CONFIG_HOME:-$HOME/.config}/git/credentials"
> +	test -z "$contents"
> +	read contents < "$HOME/.git-credentials"
> +	test -z "$contents"
> +'

I'd rather do stg like

echo >expected test_cmp expected "${XDG_CONFIG_HOME:-$HOME/.config}/git/credentials" test_cmp expected $HOME/.git-credentials

so that a test failure shows a diagnosis.

Regards,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/