[FFmpeg-devel] [PATCH] Add tests for functions in hash.c

[Reimar Döffinger]

On Wed, Mar 09, 2016 at 10:27:29AM -0800, NagaChaitanya Vellanki wrote:
> On Tue, Mar 8, 2016 at 5:33 PM, James Almer <jamrial at gmail.com> wrote:
> 
> > On 3/8/2016 2:21 AM, NagaChaitanya Vellanki wrote:
> > > ---
> > >  libavutil/Makefile       |  1 +
> > >  libavutil/hash.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
> > >  tests/fate/libavutil.mak |  4 ++++
> > >  tests/ref/fate/hash      | 45
> > +++++++++++++++++++++++++++++++++++++++++++++
> > >  4 files changed, 92 insertions(+)
> > >  create mode 100644 tests/ref/fate/hash
> >
> > LGTM. I'll let the maintainer give the final ok and push it (CCing him as
> > well).

It looks fine to me, though there are a few things that could
be improved if wanted.
For example all-0s for src is a bit boring, just a
couple of random numbers would be a better test.
At 64 bytes it is also a bit short, for some hashes that
would mean the same size as the hash, and thus some
parts of the algorithm would never ever run.
A power-of-two size also means the padding special-cases
are not tested.
Something like e.g. 133 bytes might be a better test
(assuming all our hashes support that as input?
I actually have no idea who is responsible for padding).
As we are testing things, changing the memset to
not 0 but e.g. 'a' would ensure that we would notice
if someone broke then 0-termination of the string,
and it should then also be run before the av_hash_final_b64.
But regardless of that it is great to have a test,
these are just some improvement ideas that also
can be added later.