Re: svn commit: r1028104 - /subversion/branches/performance/subversion/tests/libsvn_subr/string -test.c

Daniel Shahaf wrote:
> +1
>
> stefan2_at_apache.org wrote on Wed, Oct 27, 2010 at 21:23:35 -0000:
> > URL: http://svn.apache.org/viewvc?rev=1028104&view=rev
> > Log:
> > Adapt string unit test to recent behavioral changes.
> >
> > * subversion/tests/libsvn_subr/string-test.c
> > (test10): relax tests on string capacity

Sure, +1 to your changes here.

But what are the rest of these crazy "requirements" in this old test?

[...]
> > /* Test that:
> > - * - The initial block was just the right fit.
> > + * - The initial block was at least the right fit.
> > + * - The initial block was not excessively large.

Yup, great.

> > * - The block more than doubled (because second string so long).

This works, for typical alignments and this test data. But "more than
doubled" is not necessary. A sensible test would be that it "at least
doubled".

> > * - The block grew by a power of 2.

Why would we care whether it grew by a power of 2? Any growth by at
least a factor of 2 is efficient and satisfactory.

> > */
> > - if ((len_1 == (block_len_1 - 1))
> > - && ((block_len_2 / block_len_1) > 2)
> > - && (((block_len_2 / block_len_1) % 2) == 0))
> > + if ((len_1 <= (block_len_1 - 1))
> > + && ((block_len_1 - len_1) <= APR_ALIGN_DEFAULT(1))
> > + && ((block_len_2 / block_len_1) > 2)
> > + && (((block_len_2 / block_len_1) % 2) == 0))

That last line does NOT check that the block length grew by a power of
two anyway. It checks it grew by a factor of [2 to 2.9999] or [4 to
4.9999] or [6 to 6.9999] or ...

Let's axe that last line.

- Julian

> > return SVN_NO_ERROR;
> > else
> > return fail(pool, "test failed");
Received on 2010-10-28 16:53:24 CEST