Re: svn commit: r1424469 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py

On Wed, Jan 2, 2013 at 5:44 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> On Fri, Dec 21, 2012 at 4:47 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>> On Fri, Dec 21, 2012 at 1:10 PM, Paul Burba <ptburba_at_gmail.com> wrote:
>>> On Fri, Dec 21, 2012 at 11:34 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>>>> The problem is that I didn’t break real changes, but only changed the
>>>> reporting of entry-prop changes (think: last_*) as real changes.
>>>>
>>>> That leaves a probable real bug somewhere else where we skip the processing
>>>> of the real changes (somewhere below the entry prop changes) without even
>>>> looking at them.
>>>>
>>>> This resolved many cases where there were no real changes (most testcases
>>>> didn't document changes on the nodes that were reported as changes before),
>>>> but as you confirmed also showed that we somehow relied on seeing these non
>>>> changes.
>>>>
>>>> Paul, can you reinstate the the cases where there are true changes as XFail
>>>> tests so I can start looking where we skip the real (property or text)
>>>> changes.
>>>
>>> Hi Bert,
>>>
>>> Done in r1425065 for the merge and merge_reintegrate tests.
>>>
>>> I still need to dig into merge_tree_conflict_tests.py 20 'tree
>>> conflicts: tree missing, leaf edit' to see what exactly it's doing,
>>> but I need to step out for a moment. I'll look at that when I return.
>>
>> Hey Bert - I switched the these two tests back to their old
>> expectations too in r1425157. Even though no merge tracking is going
>> on, we are now only reporting a path as skipped if it is the root of
>> the missing subtree (missing due to OS-level deletion). The paths you
>> removed from the skip notifications *do* have operative changes
>> incoming.
>>
>> From your private email it sounds like you'll be tackling this in the
>> short term. If not we should file an issue and associate these
>> various tests with it so this regression doesn't slip through the
>> cracks.
>
> Hi Bert,
>
> I looked at the changes you made to the aforementioned tests (as well
> as all the merge_* tests) in r1426792, r1426786, r1425365, r1425362,
> and r1425271. Everything looks correct to me.
>
> The only oddity I noticed was that
> svntest.actions.run_and_verify_merge() doesn't handle expected skips
> as I would expect. For example, if 'D\H\omega' and 'D' are both
> reported as skipped by a merge, we only need to pass 'D\H\omega' as
> the expected skip and the test passes. We can also pass 'D', that
> passes too! See the comment patch below for an example of this in the
> test suite.
>
> I'm not sure if expected skips have always behaved this way. I'll
> look into it tomorrow.

Taught the test suite to correctly detect nested skip notifications in r1428996.

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> [[[
> Index: subversion/tests/cmdline/merge_tests.py
> ===================================================================
> --- subversion/tests/cmdline/merge_tests.py     (revision 1427984)
> +++ subversion/tests/cmdline/merge_tests.py     (working copy)
> @@ -7875,6 +7875,23 @@
>      'mu'        : Item("New content",
>                         props={SVN_PROP_MERGEINFO : '/A/mu:5-9'}),
>      })
> +  # Why isn't 'D' in the expected skip output?  And more importantly,
> +  # why doesn't this test fail without it?
> +  #
> +  #  >svn merge ^^/A -r4:9 .
> +  #  Skipped 'B\E\beta'
> +  #  --- Merging r5 through r9 into 'mu':
> +  #  U    mu
> +  #  Skipped 'D\H\omega'
> +  #  Skipped 'D'
> +  #  --- Merging r5 through r9 into '.':
> +  #   U   .
> +  #  --- Recording mergeinfo for merge of r5 through r9 into '.':
> +  #   U   .
> +  #  --- Recording mergeinfo for merge of r5 through r9 into 'mu':
> +  #   U   mu
> +  #  Summary of conflicts:
> +  #    Skipped paths: 3
>    expected_skip = svntest.wc.State(files_dir, {
>      'D/H/omega'       : Item(),
>      'B/E/beta'        : Item(),
> @@ -7923,6 +7940,7 @@
>      'D/H/omega'        : Item(),
>      'B/E/beta'         : Item(),
>      })
> +  raise svntest.Failure("PTB")
>    svntest.actions.run_and_verify_merge(empty_dir, '4', '9',
>                                         sbox.repo_url + '/A', None,
>                                         expected_output,
> ]]]
>
>
>
>
>> Thanks,
>>
>> --
>> Paul T. Burba
>> CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
>> Skype: ptburba