Re: [PATCH v4 01/11] add fetch-pack --diag-url tests for some corner cases

[Junio C Hamano <gitster@pobox.com>]

Mike Hommey <mh@glandium.org> writes:

> t5603-clone-dirname uses url patterns that are not tested with
> fetch-pack --diag-url, and it would be useful if they were.
>
> Interestingly, some of those tests, involving both a port and a
> user:password pair, don't currently pass. Note that even if a
> user:password pair is actually not supported by git, the values used
> could be valid user names (user names can actually contain colons
> and at signs), and are still worth testing the url parser for.

I am not sure about the last part of this (and the tests in the
patch for them).  When you are constrained by the Common Internet
Scheme Syntax, i.e.

    <scheme>://<user>:<password>@<host>:<port>/<url-path>

you cannot have arbitrary characters in these parts; within the user
and password field, any ":", "@", or "/" must be encoded.

Which maens that for the purpose of the parser you are modifying,
you can rely on these three special characters to parse things out
(decoding after the code determines which part is user and which
part is password is a separate issue).

> Signed-off-by: Mike Hommey <mh@glandium.org>
> ---
>  t/t5500-fetch-pack.sh | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
>
> Note I'm not /entirely/ sure about colons in user names, but ssh happily
> sends requests to authenticate with logins containing colons. I however
> *do* know it works with at signs (hg.mozilla.org ssh accounts are email
> addresses).
>
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index e5f83bf..1f0133f 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -569,12 +569,27 @@ check_prot_host_port_path () {
>  	test_cmp expected actual
>  }
>  
> -for r in repo re:po re/po
> +test_maybe_fail () {

That is way too confusing a name when reading the caller of it by
being too close to generic test helpers like test_might_fail,
test_must_fail, etc.

> +	host=$1; shift
> +	case $host in
> +		git=*)

Dedent this line by one level.

> +		test_expect_success "$@"
> +		;;

These two lines are indented correctly.