Re: [RFC/PATCH] t5525: test the tagopt variable and that it can be overridden
On Fri, Aug 13, 2010 at 20:13, Daniel Johnson <computerdruid@gmail.com> wrote:
Show 9 quoted lines
> ---
>> The current behaviour seems to me a bug introduced while git-fetch was
>> rewritten in C (the original found in contrib/examples reads from the
>> config only when no --tags/--no-tags option is given from the command
>> line).
>>
>> Is this something we can protect with a test script from future breakages?
> This should test that behavior. I'd appreciate feedback on how to improve this
> test. I'm not sure if this is the right name/number either.
Thanks for tackling this.
Show 5 quoted lines
> t/t5525-fetch-tagopt.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 44 insertions(+), 0 deletions(-)
> create mode 100755 t/t5525-fetch-tagopt.sh
>
> diff --git a/t/t5525-fetch-tagopt.sh b/t/t5525-fetch-tagopt.sh
The test name looks fine.
(The t55* test names are a bit of a mess with mixed pull/fetch, but that's not something that should be dealt with here)
Show 7 quoted lines
> new file mode 100755
> index 0000000..17bd407
> --- /dev/null
> +++ b/t/t5525-fetch-tagopt.sh
> @@ -0,0 +1,44 @@
> +
> +#!/bin/sh
Is that an empty line before the test begins? The shebang should be on the first line.
Show 10 quoted lines
> +test_description='tagopt variable affects "git fetch" and is overridden by commandline.'
> +
> +. ./test-lib.sh
> +
> +setup_clone () {
> + (git clone --mirror . $1 &&
> + git remote add remote_$1 $1 &&
> + cd $1 &&
> + git tag tag_$1)
> +}Maybe only put the "cd $1 ..." inside a subshell for clarity.
> +test_expect_success setup '
> + echo >file original &&
> + git add file &&
> + git commit -a -m original &&
Maybe this can use test_commit if you don't mind it creating a tag too.
Show 10 quoted lines
> + setup_clone one &&
> + git config remote.remote_one.tagopt --no-tags &&
> + setup_clone two &&
> + git config remote.remote_two.tagopt --tags
> + '
> +
> +test_expect_success "fetch with tagopt=--no-tags does not get tag" '
> + git fetch remote_one &&
> + ! (git show-ref tag_one)
> + '
Doesn't need a subshell? You should also use:
test_must_fail git show-ref ...
> +test_expect_success "fetch --tags with tagopt=--no-tags gets tag" '
> + git fetch --tags remote_one &&
> + (git show-ref tag_one)
> + '
Show 10 quoted lines
> +test_expect_success "fetch --no-tags with tagopt=--tags does not get tag" '
> + git fetch --no-tags remote_two &&
> + ! (git show-ref tag_two)
> + '
> +
> +test_expect_success "fetch with tagopt=--tags gets tag" '
> + git fetch remote_two &&
> + (git show-ref tag_two)
> + '
> +test_done
test_must_fail etc etc.
Otherwise it looks good.
Daniel Johnson
