You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2010/12/22 07:15:53 UTC
Re: svn commit: r1051745 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
On 12/22/2010 09:38 AM, blair@apache.org wrote:
> Author: blair
> Date: Wed Dec 22 04:08:14 2010
> New Revision: 1051745
>
> URL: http://svn.apache.org/viewvc?rev=1051745&view=rev
> Log:
> Update test_commit_txn() to handle svn_fs_commit_txn()'s return
> semantics.
>
> * subversion/tests/libsvn_fs/fs-test.c
> (test_commit_txn):
> If svn_fs_commit_txn() returns an error, then always return an
> error to the caller, just use a different wrapping error message
> if the commit succeeded or failed.
> If svn_fs_commit_txn() returns no error, than assert that a valid
> revision number was returned.
>
> Modified:
> subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>
> Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1051745&r1=1051744&r2=1051745&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Wed Dec 22 04:08:14 2010
> @@ -61,6 +61,7 @@
> * EXPECTED_CONFLICT. If they don't match, return error.
> *
> * If a conflict is expected but the commit succeeds anyway, return
> + * error. If the commit fails but does not provide an error, return
> * error.
> */
> static svn_error_t *
> @@ -110,13 +111,24 @@ test_commit_txn(svn_revnum_t *new_rev,
> "conflicting commit returned valid new revision");
> }
> }
> - else if (err) /* commit failed, but not due to conflict */
> + else if (err) /* commit may have succeeded, but always report an error */
> {
> - return svn_error_quick_wrap
> - (err, "commit failed due to something other than a conflict");
> + if (SVN_IS_VALID_REVNUM(*new_rev))
> + return svn_error_quick_wrap
> + (err, "commit succeeded but something else failed");
Should this error not be wrapped inside "_()"?
> + else
> + return svn_error_quick_wrap
> + (err, "commit failed due to something other than a conflict");
Same as above.
> }
> - else /* err == NULL, so commit succeeded */
> + else /* err == NULL, commit should have succeeded */
> {
> + if (! SVN_IS_VALID_REVNUM(*new_rev))
> + {
> + return svn_error_create
> + (SVN_ERR_FS_GENERAL, NULL,
> + "commit failed but no error was returned");
Same as above.
Thanks
With regards
Kamesh Jayachandran
Re: svn commit: r1051745 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 12/22/2010 07:21 PM, Hyrum K. Wright wrote:
> On Wed, Dec 22, 2010 at 1:15 AM, Kamesh Jayachandran<ka...@collab.net> wrote:
>> On 12/22/2010 09:38 AM, blair@apache.org wrote:
>>> Author: blair
>>> Date: Wed Dec 22 04:08:14 2010
>>> New Revision: 1051745
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1051745&view=rev
>>> Log:
>>> Update test_commit_txn() to handle svn_fs_commit_txn()'s return
>>> semantics.
>>>
>>> * subversion/tests/libsvn_fs/fs-test.c
>>> (test_commit_txn):
>>> If svn_fs_commit_txn() returns an error, then always return an
>>> error to the caller, just use a different wrapping error message
>>> if the commit succeeded or failed.
>>> If svn_fs_commit_txn() returns no error, than assert that a valid
>>> revision number was returned.
>>>
>>> Modified:
>>> subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>>>
>>> Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>>> URL:
>>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1051745&r1=1051744&r2=1051745&view=diff
>>>
>>> ==============================================================================
>>> --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
>>> +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Wed Dec 22
>>> 04:08:14 2010
>>> @@ -61,6 +61,7 @@
>>> * EXPECTED_CONFLICT. If they don't match, return error.
>>> *
>>> * If a conflict is expected but the commit succeeds anyway, return
>>> + * error. If the commit fails but does not provide an error, return
>>> * error.
>>> */
>>> static svn_error_t *
>>> @@ -110,13 +111,24 @@ test_commit_txn(svn_revnum_t *new_rev,
>>> "conflicting commit returned valid new revision");
>>> }
>>> }
>>> - else if (err) /* commit failed, but not due to conflict */
>>> + else if (err) /* commit may have succeeded, but always report an
>>> error */
>>> {
>>> - return svn_error_quick_wrap
>>> - (err, "commit failed due to something other than a conflict");
>>> + if (SVN_IS_VALID_REVNUM(*new_rev))
>>> + return svn_error_quick_wrap
>>> + (err, "commit succeeded but something else failed");
>> Should this error not be wrapped inside "_()"?
>>
>>> + else
>>> + return svn_error_quick_wrap
>>> + (err, "commit failed due to something other than a conflict");
>> Same as above.
>>
>>> }
>>> - else /* err == NULL, so commit succeeded */
>>> + else /* err == NULL, commit should have succeeded */
>>> {
>>> + if (! SVN_IS_VALID_REVNUM(*new_rev))
>>> + {
>>> + return svn_error_create
>>> + (SVN_ERR_FS_GENERAL, NULL,
>>> + "commit failed but no error was returned");
>> Same as above.
> Do we ask our translators to translate error messages specific to the
> testsuite? (Honest question; I really don't know.)
>
> If we do, I'd propose we stop. :) Their time is better spent
> localizing the core product, not the tests. (And none of the Python
> test suite is localized, iirc.)
Sorry I failed to notice this as *test*.
With regards
Kamesh Jayachandran
> -Hyrum
Re: svn commit: r1051745 - /subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
On Wed, Dec 22, 2010 at 1:15 AM, Kamesh Jayachandran <ka...@collab.net> wrote:
> On 12/22/2010 09:38 AM, blair@apache.org wrote:
>>
>> Author: blair
>> Date: Wed Dec 22 04:08:14 2010
>> New Revision: 1051745
>>
>> URL: http://svn.apache.org/viewvc?rev=1051745&view=rev
>> Log:
>> Update test_commit_txn() to handle svn_fs_commit_txn()'s return
>> semantics.
>>
>> * subversion/tests/libsvn_fs/fs-test.c
>> (test_commit_txn):
>> If svn_fs_commit_txn() returns an error, then always return an
>> error to the caller, just use a different wrapping error message
>> if the commit succeeded or failed.
>> If svn_fs_commit_txn() returns no error, than assert that a valid
>> revision number was returned.
>>
>> Modified:
>> subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>>
>> Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1051745&r1=1051744&r2=1051745&view=diff
>>
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
>> +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Wed Dec 22
>> 04:08:14 2010
>> @@ -61,6 +61,7 @@
>> * EXPECTED_CONFLICT. If they don't match, return error.
>> *
>> * If a conflict is expected but the commit succeeds anyway, return
>> + * error. If the commit fails but does not provide an error, return
>> * error.
>> */
>> static svn_error_t *
>> @@ -110,13 +111,24 @@ test_commit_txn(svn_revnum_t *new_rev,
>> "conflicting commit returned valid new revision");
>> }
>> }
>> - else if (err) /* commit failed, but not due to conflict */
>> + else if (err) /* commit may have succeeded, but always report an
>> error */
>> {
>> - return svn_error_quick_wrap
>> - (err, "commit failed due to something other than a conflict");
>> + if (SVN_IS_VALID_REVNUM(*new_rev))
>> + return svn_error_quick_wrap
>> + (err, "commit succeeded but something else failed");
>
> Should this error not be wrapped inside "_()"?
>
>> + else
>> + return svn_error_quick_wrap
>> + (err, "commit failed due to something other than a conflict");
>
> Same as above.
>
>> }
>> - else /* err == NULL, so commit succeeded */
>> + else /* err == NULL, commit should have succeeded */
>> {
>> + if (! SVN_IS_VALID_REVNUM(*new_rev))
>> + {
>> + return svn_error_create
>> + (SVN_ERR_FS_GENERAL, NULL,
>> + "commit failed but no error was returned");
>
> Same as above.
Do we ask our translators to translate error messages specific to the
testsuite? (Honest question; I really don't know.)
If we do, I'd propose we stop. :) Their time is better spent
localizing the core product, not the tests. (And none of the Python
test suite is localized, iirc.)
-Hyrum