On 12/27/10 11:49 AM, Daniel Shahaf wrote:
> blair_at_apache.org wrote on Mon, Dec 27, 2010 at 19:04:39 -0000:
>> Author: blair
>> Date: Mon Dec 27 19:04:39 2010
>> New Revision: 1053140
>>
>> URL: http://svn.apache.org/viewvc?rev=1053140&view=rev
>> Log:
>> Add a unit test for svn_error_purge_tracing() that asserts that
>> SVN_ERR_ASSERT() fails when an error chain with only tracing errors is
>> passed in, which shouldn't happen in practice.
>>
>> * subversion/tests/libsvn_subr/error-test.c
>> (test_error_purge_tracing):
>> Only if SVN_ERR__TRACING is defined, build an error chain with
>> only tracing errors and pass it to svn_error_purge_tracing(),
>> asserting that it returns a new error chain with a
>> SVN_ERR_ASSERTION_FAIL error.
>>
>> Modified:
>> subversion/trunk/subversion/tests/libsvn_subr/error-test.c
>>
>> Modified: subversion/trunk/subversion/tests/libsvn_subr/error-test.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/error-test.c?rev=1053140&r1=1053139&r2=1053140&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/libsvn_subr/error-test.c (original)
>> +++ subversion/trunk/subversion/tests/libsvn_subr/error-test.c Mon Dec 27 19:04:39 2010
>> @@ -111,8 +111,54 @@ test_error_purge_tracing(apr_pool_t *poo
>> "Tracing link found after purging the "
>> "following chain:");
>> }
>> -
>> svn_error_clear(err);
>> +
>> +#ifdef SVN_ERR__TRACING
>> + /* Make an error chain containing only tracing errors and check that
>> + svn_error_purge_tracing() asserts on it. */
>> + {
>> + svn_error_t err_copy, err2_copy;
>> + svn_error_malfunction_handler_t orig_handler;
>> +
>> + /* For this test, use a random error status. */
>> + err = svn_error_create(SVN_ERR_BAD_UUID, NULL, SVN_ERR__TRACED);
>> + err = svn_error_return(err);
>> +
>> + /* Register a malfunction handler that doesn't call abort() to
>> + check that a new error chain with a SVN_ERR_ASSERTION_FAIL is
>> + returned. */
>> + orig_handler =
>> + svn_error_set_malfunction_handler(svn_error_raise_on_malfunction);
>> + err2 = svn_error_purge_tracing(err);
>> + svn_error_set_malfunction_handler(orig_handler);
>> +
>> + err_copy = *err;
>> +
>> + if (err2)
>> + {
>> + /* If err2 does share the same pool as err, then make a copy
>> + of err2 before err is cleared. */
>> + err2_copy = *err2;
>> +
>> + svn_error_clear(err);
>> +
>> + /* The returned error is only safe to clear if this assertion
>> + holds, otherwise it has the same pool as the original
>> + error. */
>> + SVN_ERR_ASSERT(err_copy.pool != err2_copy.pool);
>> +
>> + svn_error_clear(err2);
>> +
>> + SVN_ERR_ASSERT(SVN_ERR_ASSERTION_FAIL == err2_copy.apr_err);
>> + }
>> + else
>> + {
>> + svn_error_clear(err);
>> + SVN_ERR_ASSERT(err2);
>
> The last assert will trigger whenever the else{} block is reached, is
> that your intention?
Yes, because err2 should never be equal to SVN_NO_ERROR. I'm just managing the
error chain here.
> Also: are you aware of SVN_TEST_ASSERT()?
Nope, I've updated the code to use SVN_TEST_ASSERT() in r1053174.
> Also: formally, if you just got an SVN_ERR_ASSERTION_FAIL, the API
> contract doesn't allow you to continue calling into svn libraries. In
> other words, unless you verify that the "expected" assertion was
> triggered, some day we might change the code, the test will hit some
> other assertion --- and will happily ignore it :(
Where is this documented? That doesn't make sense in a multi-threaded context
as one would have to stop all threads.
There's always a chance we may catch another error if we change the code, don't
know if there's much to do about that.
Blair
Received on 2010-12-27 21:48:05 CET