You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2011/03/06 21:14:36 UTC

Re: svn commit: r1076726 - /subversion/trunk/subversion/tests/cmdline/log_tests.py

pburba@apache.org wrote on Thu, Mar 03, 2011 at 19:09:01 -0000:
> Author: pburba
> Date: Thu Mar  3 19:09:01 2011
> New Revision: 1076726
> 
> URL: http://svn.apache.org/viewvc?rev=1076726&view=rev
> Log:
> * subversion/tests/cmdline/log_tests.py
>   (check_merge_results): Don't assume expected_reverse_merges is present, it
>    may be None.
> 
> Modified:
>     subversion/trunk/subversion/tests/cmdline/log_tests.py
> 
> Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1076726&r1=1076725&r2=1076726&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Thu Mar  3 19:09:01 2011
> @@ -1149,7 +1149,8 @@ def check_merge_results(log_chain, expec
>    # Check to see if the number and values of the revisions is correct
>    for log in log_chain:
>      if (log['revision'] not in expected_merges
> -        and log['revision'] not in expected_reverse_merges):
> +        and (expected_reverse_merges is not None
> +             and log['revision'] not in expected_reverse_merges)):

I'm re-reading this and I'm still not convinced that it's correct:
it means that if expected_reverse_merges is None, then the "Found
unexpected revision" error will never be raised.  Is that the intended
semantics?

Or did you mean this ---

     if (log['revision'] not in expected_merges
         and (expected_reverse_merges is not None
              ? log['revision'] not in expected_reverse_merges
              : True):

>        raise SVNUnexpectedLogs("Found unexpected revision %d" %
>                                log['revision'], log_chain)
>  
> 
> 

Re: svn commit: r1076726 - /subversion/trunk/subversion/tests/cmdline/log_tests.py

Posted by Paul Burba <pt...@gmail.com>.
On Sun, Mar 6, 2011 at 3:14 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> pburba@apache.org wrote on Thu, Mar 03, 2011 at 19:09:01 -0000:
>> Author: pburba
>> Date: Thu Mar  3 19:09:01 2011
>> New Revision: 1076726
>>
>> URL: http://svn.apache.org/viewvc?rev=1076726&view=rev
>> Log:
>> * subversion/tests/cmdline/log_tests.py
>>   (check_merge_results): Don't assume expected_reverse_merges is present, it
>>    may be None.
>>
>> Modified:
>>     subversion/trunk/subversion/tests/cmdline/log_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1076726&r1=1076725&r2=1076726&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Thu Mar  3 19:09:01 2011
>> @@ -1149,7 +1149,8 @@ def check_merge_results(log_chain, expec
>>    # Check to see if the number and values of the revisions is correct
>>    for log in log_chain:
>>      if (log['revision'] not in expected_merges
>> -        and log['revision'] not in expected_reverse_merges):
>> +        and (expected_reverse_merges is not None
>> +             and log['revision'] not in expected_reverse_merges)):
>
> I'm re-reading this and I'm still not convinced that it's correct:
> it means that if expected_reverse_merges is None, then the "Found
> unexpected revision" error will never be raised.  Is that the intended
> semantics?

Hi Daniel,

 You are quite right, that error will never be raised and we can get a
false pass if the expected revisions are not complete.  Also, this
function needs to handle the possibility of EXPECTED_MERGES being set
to none.  Fixed both of these in
http://svn.apache.org/viewvc?view=revision&revision=1079400

Thanks,

Paul


> Or did you mean this ---
>
>     if (log['revision'] not in expected_merges
>         and (expected_reverse_merges is not None
>              ? log['revision'] not in expected_reverse_merges
>              : True):
>
>>        raise SVNUnexpectedLogs("Found unexpected revision %d" %
>>                                log['revision'], log_chain)
>>
>>
>>
>

Re: svn commit: r1076726 - /subversion/trunk/subversion/tests/cmdline/log_tests.py

Posted by Paul Burba <pt...@gmail.com>.
On Sun, Mar 6, 2011 at 3:14 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> pburba@apache.org wrote on Thu, Mar 03, 2011 at 19:09:01 -0000:
>> Author: pburba
>> Date: Thu Mar  3 19:09:01 2011
>> New Revision: 1076726
>>
>> URL: http://svn.apache.org/viewvc?rev=1076726&view=rev
>> Log:
>> * subversion/tests/cmdline/log_tests.py
>>   (check_merge_results): Don't assume expected_reverse_merges is present, it
>>    may be None.
>>
>> Modified:
>>     subversion/trunk/subversion/tests/cmdline/log_tests.py
>>
>> Modified: subversion/trunk/subversion/tests/cmdline/log_tests.py
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/log_tests.py?rev=1076726&r1=1076725&r2=1076726&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/tests/cmdline/log_tests.py (original)
>> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Thu Mar  3 19:09:01 2011
>> @@ -1149,7 +1149,8 @@ def check_merge_results(log_chain, expec
>>    # Check to see if the number and values of the revisions is correct
>>    for log in log_chain:
>>      if (log['revision'] not in expected_merges
>> -        and log['revision'] not in expected_reverse_merges):
>> +        and (expected_reverse_merges is not None
>> +             and log['revision'] not in expected_reverse_merges)):
>
> I'm re-reading this and I'm still not convinced that it's correct:
> it means that if expected_reverse_merges is None, then the "Found
> unexpected revision" error will never be raised.  Is that the intended
> semantics?

Hi Daniel,

 You are quite right, that error will never be raised and we can get a
false pass if the expected revisions are not complete.  Also, this
function needs to handle the possibility of EXPECTED_MERGES being set
to none.  Fixed both of these in
http://svn.apache.org/viewvc?view=revision&revision=1079400

Thanks,

Paul


> Or did you mean this ---
>
>     if (log['revision'] not in expected_merges
>         and (expected_reverse_merges is not None
>              ? log['revision'] not in expected_reverse_merges
>              : True):
>
>>        raise SVNUnexpectedLogs("Found unexpected revision %d" %
>>                                log['revision'], log_chain)
>>
>>
>>
>