On Tue, Mar 15, 2011 at 9:56 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> Paul Burba wrote on Tue, Mar 15, 2011 at 09:38:31 -0400:
>> On Tue, Mar 15, 2011 at 12:24 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>> > pburba_at_apache.org wrote on Tue, Mar 08, 2011 at 15:46:10 -0000:
>> >> +++ subversion/trunk/subversion/tests/cmdline/log_tests.py Tue Mar 8 15:46:09 2011
>> >> @@ -1148,9 +1148,9 @@ 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 (expected_reverse_merges is not None
>> >> - and log['revision'] not in expected_reverse_merges)):
>> >> + if not ((expected_merges and log['revision'] in expected_merges)
>> >> + or (expected_reverse_merges
>> >> + and log['revision'] in expected_reverse_merges)):
>> >
>> > If EXPECTED_MERGES and EXPECTED_REVERSE_MERGES are both None,
>> > then the if() would trigger --- and I don't think that's the
>> > intention.
>>
>> Hi Daniel,
>>
>> It is the intention. If EXPECTED_MERGES and EXPECTED_REVERSE_MERGES
>> are both None, then the caller believes that no merged revisions
>> (normal or reverse) are present. However, there *is* something in the
>> LOG_CHAIN, so there is an error. Admittedly, none of the present
>> callers pass EXPECTED_MERGES=None and EXPECTED_REVERSE_MERGES=None,
>> but we might have reason to do so in the future.
>>
>> Paul
Possibly I'm abusing a Python convention, but...
> I see; I assumed that passing None means "I don't care about this piece
> of information; do not attempt to validate it",
That makes sense for a function like
svntest/actions.py:run_and_verify_svn2(), where if EXPECTED_STDOUT is
None, we don't check stdout. That function's core purpose is to
invoke main.run_svn(). The caller may or may not care about
confirming the stdout, so EXPECTED_STDOUT is justifiably optional.
On the other hand, log_tests.py:check_merge_results() has only one
purpose, to check that the expected '(Reverse) Merged via' headers
match the actual output. That's it. Why would we ever want to ignore
either? All that would enable us to do is write a test that
spuriously passes.
Paul
> and that callers who
> believe there are no merged revisions would pass an empty dict/list.
>
> Thanks for the clarification,
>
> Daniel
>
Received on 2011-03-15 15:47:51 CET