On Thu, 2010-10-28 at 15:52 +0100, Julian Foad wrote:
> 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.
Done in r1028340.
Apologies in advance for the merge conflict this will cause when merging
your change to trunk.
- Julian
Received on 2010-10-28 17:07:36 CEST