Re: [PATCH] t3200-branch: test setting branch as own upstream

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

 



On Wed, Mar 05, 2014 at 04:31:55PM +0900, Brian Gesiak wrote:

> No test asserts that "git branch -u refs/heads/my-branch my-branch"
> emits a warning. Add a test that does so.
> 
> Signed-off-by: Brian Gesiak <modocache@xxxxxxxxx>

Thanks, this looks good. Two minor points that may or may not be worth
addressing:

> +test_expect_success '--set-upstream-to shows warning if used to set branch as own upstream' '
> +	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
> +	cat >expected <<EOF &&
> +warning: Not setting branch my13 as its own upstream.
> +EOF

If you spell the EOF marker as:

    cat >expect <<-\EOF

then:

  1. The shell does not interpolate the contents (it does not matter
     here, but it is a good habit to be in, so we typically do it unless
     there is a need to interpolate).

  2. Using <<- will strip leading tabs, so the content can be indented
     properly along with the rest of the test.

> +	test_i18ncmp expected actual &&
> +	test_must_fail git config branch.my13.remote &&
> +	test_must_fail git config branch.my13.merge

I think we could tighten these to:

  test_expect_code 1 git config branch.my13.remote

to eliminate a false-positive success on other config errors. It's
highly improbable for it to ever matter, though (and it looks like we
are not so careful in most other places that call "git config" looking
for a missing entry, either).

-Peff
--
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]