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

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

 



Lawrence Siebert <lawrencesiebert@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  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/
--
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]