You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2010/10/27 23:23:35 UTC
svn commit: r1028104 -
/subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c
Author: stefan2
Date: Wed Oct 27 21:23:34 2010
New Revision: 1028104
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
Modified:
subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c
Modified: subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c?rev=1028104&r1=1028103&r2=1028104&view=diff
==============================================================================
--- subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c (original)
+++ subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c Wed Oct 27 21:23:34 2010
@@ -231,13 +231,15 @@ test10(apr_pool_t *pool)
block_len_2 = (s->blocksize);
/* 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.
* - The block more than doubled (because second string so long).
* - The block grew by a power of 2.
*/
- 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))
return SVN_NO_ERROR;
else
return fail(pool, "test failed");
Re: svn commit: r1028104 -
/subversion/branches/performance/subversion/tests/libsvn_subr/string
-test.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sat, Oct 30, 2010 at 18:27:40 +0200:
> On 28.10.2010 17:06, Julian Foad wrote:
>> Apologies in advance for the merge conflict this will cause when merging
>> your change to trunk.
> No worries.
>
> BTW, is there any policy on documenting merge conflict
> so that people can have a second look at it if they care?
>
Just say in the log message where/what the conflicts were and what
approach you took to resolve them, I suppose.
In the past, when merging to cherry-pick branches (/branches/1.6.x-rN),
we have precedent for doing the merge, committing the merged result
*with* conflict markers, and then resolving in a separate revision.
Though, if you do this on trunk, you'll just break the build, so it's
not as good an option in this case.
Re: svn commit: r1028104 - /subversion/branches/performance/subversion/tests/libsvn_subr/string
-test.c
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 28.10.2010 17:06, Julian Foad wrote:
> On Thu, 2010-10-28 at 15:52 +0100, Julian Foad wrote:
>> Daniel Shahaf wrote:
>>> +1
>>>
>>> stefan2@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.
No worries.
BTW, is there any policy on documenting merge conflict
so that people can have a second look at it if they care?
-- Stefan^2.
Re: svn commit: r1028104 -
/subversion/branches/performance/subversion/tests/libsvn_subr/string -test.c
Posted by Julian Foad <ju...@wandisco.com>.
On Thu, 2010-10-28 at 15:52 +0100, Julian Foad wrote:
> Daniel Shahaf wrote:
> > +1
> >
> > stefan2@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
Re: svn commit: r1028104 -
/subversion/branches/performance/subversion/tests/libsvn_subr/string -test.c
Posted by Julian Foad <ju...@wandisco.com>.
Daniel Shahaf wrote:
> +1
>
> stefan2@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");
Re: svn commit: r1028104 -
/subversion/branches/performance/subversion/tests/libsvn_subr/string
-test.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
+1
stefan2@apache.org wrote on Wed, Oct 27, 2010 at 21:23:35 -0000:
> Author: stefan2
> Date: Wed Oct 27 21:23:34 2010
> New Revision: 1028104
>
> 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
>
> Modified:
> subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c
>
> Modified: subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c?rev=1028104&r1=1028103&r2=1028104&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c (original)
> +++ subversion/branches/performance/subversion/tests/libsvn_subr/string-test.c Wed Oct 27 21:23:34 2010
> @@ -231,13 +231,15 @@ test10(apr_pool_t *pool)
> block_len_2 = (s->blocksize);
>
> /* 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.
> * - The block more than doubled (because second string so long).
> * - The block grew by a power of 2.
> */
> - 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))
> return SVN_NO_ERROR;
> else
> return fail(pool, "test failed");
>
>