From: Torsten Bögershausen <tboegi@web.de>
Date: Thu, 07 May 2015 09:05:47 GMT
Subject: Re: [PATCH v2 01/12] t5520: implement tests for no merge candidates cases
Message-ID: <554B2AEB.3020608@web.de>
In-Reply-To: <1430988248-18285-2-git-send-email-pyokagan@gmail.com>

On 05/07/2015 10:43 AM, Paul Tan wrote:
> Commit a8c9bef4 fully established the current advices given by git-pull
> for the different cases where git-fetch will not have anything marked
> for merge:
>
> 1. We fetched from a specific remote, and a refspec was given, but it
>     ended up not fetching anything. This is usually because the user
>     provided a wildcard refspec which had no matches on the remote end.
>
> 2. We fetched from a non-default remote, but didn't specify a branch to
>     merge. We can't use the configured one because it applies to the
>     default remote, and thus the user must specify the branches to merge.
>
> 3. We fetched from the branch's or repo's default remote, but:
>
>     a. We are not on a branch, so there will never be a configured branch
>        to merge with.
>
>     b. We are on a branch, but there is no configured branch to merge
>        with.
>
> 4. We fetched from the branch's or repo's default remote, but the
>     configured branch to merge didn't get fetched (either it doesn't
>     exist, or wasn't part of the configured fetch refspec)
>
> Implement tests for the above 5 cases to ensure that the correct code
> paths are triggered for each of these cases.
>
> Signed-off-by: Paul Tan <pyokagan@gmail.com>
> ---
>
> Notes:
>      * Re-worded commit message to match the logic used in git-pull.sh's
>        error_on_no_merge_candidates().
>      
>      * The tests have thus also been reordered to match the commit message.
>      
>      * Non-hackish solution for case 3a.
>      
>      * Add more checks to ensure that git-pull does not touch any files it
>        should not be touching on failure.
>
>   t/t5520-pull.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 55 insertions(+)
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 7efd45b..5add900 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -109,6 +109,61 @@ test_expect_success 'the default remote . should not break explicit pull' '
>   	test `cat file` = modified
>   '
>   
> +test_expect_success 'fail if wildcard spec does not match any refs' '
> +	git checkout -b test copy^ &&
> +	test_when_finished "git checkout -f copy && git branch -D test" &&
> +	test `cat file` = file &&
Minor nit, please see Documentation/CodingGuidelines:
  - We prefer $( ... ) for command substitution; unlike ``, it
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
In other words:
test $(cat file) = file &&