Re: [PATCH 07/12] test-lib-functions: move function to lib-bitmap.sh

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

 



On Wed, Feb 10, 2021 at 09:56:45PM +0100, SZEDER Gábor wrote:

> > +test_bitmap_traversal () {
> > +	if test "$1" = "--no-confirm-bitmaps"
> > +	then
> > +		shift
> > +	elif cmp "$1" "$2"
> > +	then
> > +		echo >&2 "identical raw outputs; are you sure bitmaps were used?"
> > +		return 1
> 
> Since you converted two 'error's to BUG earlier in this series...
> 
> This helper function was added in ea047a8eb4 (t5310: factor out bitmap
> traversal comparison, 2020-02-14), and so I Cc: its author and quote
> part of its commit message:
> 
>     While we're at it, let's also try to confirm that the bitmap output did
>     indeed use bitmaps. Since the code internally falls back to the
>     non-bitmap path in some cases, the tests are at risk of becoming trivial
>     noops.
>     
>     This is a bit fragile, as not all outputs will differ (e.g., looking at
>     only the commits from a fully-bitmapped pack will end up exactly the
>     same as the normal traversal order, since it also matches the pack
>     order). So we'll provide an escape hatch by which tests can disable this
>     check (which should only be used after manually confirming that bitmaps
>     kicked in).
> 
> Shouldn't this be a BUG as well?  I think this should be a BUG.

I dunno. I guess you are thinking of the case where somebody adds a new
caller but fails to invoke Git correctly. Which would be a bug in the
test script.

But it could also easily be due to Git changing how it behaves in a
certain circumstance. And maybe the solution is changing the test to
adapt, or maybe we just found a bug in Git.

TBH, I do not care that much either way. Either way, I think somebody
would notice the problem.

-Peff



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

  Powered by Linux