Re: [PATCH v3 11/11] t6050-replace: use some long option names
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 1, 2013, 08:07 UTC
- Message-ID
- <CAPig+cSwQdhybTzRwqGLbdxGRcAZfBeU6jjWYy43G0oT0hxkNw@mail.gmail.com>
- In-Reply-To
- <20130831191215.26699.76207.chriscool@tuxfamily.org>
On Sat, Aug 31, 2013 at 3:12 PM, Christian Couder <chriscool@tuxfamily.org> wrote:
Show 21 quoted lines
> So that they are tested a litlle bit too.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
> t/t6050-replace.sh | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
> index 0b07a0b..5dc26e8 100755
> --- a/t/t6050-replace.sh
> +++ b/t/t6050-replace.sh
> @@ -122,9 +122,9 @@ test_expect_success '"git replace" listing and deleting' '
> test "$HASH2" = "$(git replace -l)" &&
> test "$HASH2" = "$(git replace)" &&
> aa=${HASH2%??????????????????????????????????????} &&
> - test "$HASH2" = "$(git replace -l "$aa*")" &&
> + test "$HASH2" = "$(git replace --list "$aa*")" &&
> test_must_fail git replace -d $R &&
> - test_must_fail git replace -d &&
> + test_must_fail git replace --delete &&
> test_must_fail git replace -l -d $HASH2 &&Is this sort of change a good idea? A person reading the code, but not familiar with this particular patch, might not understand the seemingly random choice of short versus long option usage. Such a person might be led to wonder if there is some subtle difference between the short and long forms, and then unnecessarily spend time digging into the code and documentation in an attempt to figure it out. Alternately, someone reading the code might be led to assume that the person who added the tests was being sloppy.
Hopefully, t0040-parse-options should already be proof enough that long options are correctly handled, but if you really want to test them here too, it seems like a separate test would be more appropriate than randomly changing short form options to long in various tests.
Show 22 quoted lines
> git replace -d $HASH2 &&
> git show $HASH2 | grep "A U Thor" &&
> @@ -147,7 +147,7 @@ test_expect_success '"git replace" resolves sha1' '
> git show $HASH2 | grep "O Thor" &&
> test_must_fail git replace $HASH2 $R &&
> git replace -f $HASH2 $R &&
> - test_must_fail git replace -f &&
> + test_must_fail git replace --force &&
> test "$HASH2" = "$(git replace)"
> '
>
> @@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects must be of the same type'
>
> test_expect_success '-f option bypasses the type check' '
> git replace -f mytag $HASH1 2>err &&
> - git replace -f HEAD^{tree} HEAD~1 2>err &&
> + git replace --force HEAD^{tree} HEAD~1 2>err &&
> git replace -f HEAD^ $BLOB 2>err
> '
>
> --
> 1.8.4.rc1.31.g530f5ce.dirty