Re: [PATCH v2 3/4] log --count: added test

[Lawrence Siebert <lawrencesiebert@gmail.com>]

Matthieu,

Ok, I'll fix that.  I think I can also add tests, I can look at the
tests for rev-list --count, with the understanding that I saw somebody
else had made changes for the  --use-bitmap-index option, and I am
basing off of master for this, and thus don't feel comfortable with
--use-bitmap-index at this time.

If it's acceptable practice, I'll just squash everything I do on this
feature and it's tests into one commit with a more detailed comment,
and send the patch for that.  I wasn't sure about how much history I
should save, and how much I should split stuff up, so I appreciate
your clarification.

Thank you for your time,
Lawrence Siebert

On Fri, Jul 3, 2015 at 12:34 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Lawrence Siebert <lawrencesiebert@gmail.com> writes:
>
>> added test comparing output between git log --count HEAD and
>> git rev-list --count HEAD
>
> Unless there is a very long list of tests, I'd rather see this squashed
> with PATCH 2/4. As a reviewer I prefer having code and tests in the same
> place.
>
>> Signed-off-by: Lawrence Siebert <lawrencesiebert@gmail.com>
>> ---
>>  t/t4202-log.sh | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
>> index 1b2e981..35f8d82 100755
>> --- a/t/t4202-log.sh
>> +++ b/t/t4202-log.sh
>> @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' '
>>       test_must_fail git log --graph --no-walk
>>  '
>>
>> +test_expect_success 'log --count' '
>> +     git log --count HEAD > actual &&
>> +     git     rev-list --count HEAD > expect &&
>
> The weird space is still there.
>
> Also, we write ">actual", not "> actual" in the Git coding style.
>
> That is actually a rather weak test. rev-list --count interacts with
> --left-right, so I guess you want to test --count --left-right.
>
> Also, some revision-limiting options can reduce the count like
>
> git log --grep whatever
>
> and you should check that you actually count the right number here.
>
> (I don't know this part of the code enough, but I'm not sure you
> actually deal with this properly)
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/



-- 
About Me: http://about.me/lawrencesiebert
Constantly Coding: http://constantcoding.blogspot.com