[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Need to resolve expected behavior of XFailing tests caused by tree conflicts (Issue #3209)

From: Stephen Butler <sbutler_at_elego.de>
Date: Thu, 05 Feb 2009 19:20:52 +0100

Quoting Mark Phippard <markphip_at_gmail.com>:

> Stephen,
>
> To clarify ...
>
> Group 3 is "done". Does that mean the test is no longer XFAIL? It
> seemed like you just changed the description of the test from the
> comments. But perhaps that was all Paul was pointing out needed to be
> done. I guess my assumption with this issue is that when we are
> "done" the tests will no longer be XFAIL.

I just meant I had improved the cryptic output of the tests (in the
docstrings).

The XFAIL parts of that test are in fairly odd situations where the
error message "A/B/C does not exist" takes precedence, or where the
update output is silent. It'd be nice to say "A is tree-conflicted"
in all of those situations, but it isn't a blocker.

The skipping behavior is changing. We may have to revise this test
soon anyway.

>
> You are working on groups 1 & 2. Do you think you'll be able to bring
> these to completion? Any ideas on how much work is left/when it will
> be completed?

I'll commit the first part of the fix in a bit, if the tests are OK.
Just making the add_file update code consistent with add_directory.
This incidentally fixes some of the issue 3209 problems Paul
mentioned.

There's a day or two of work to do, so that update/switch will tolerate
unversioned items and items added without history.

But first I think we should eliminate skipping of newly discovered tree
conflicts. Issue 3334 is more important, and involves the same code,
so I'd like to get it out of the way first.

>
> Group 4 ... what is your take on Paul's comments? Is this just an
> area where there could be differences of opinion but it is working as
> intended. Or is something broken and this is an item where more work
> needs to be done?

On looking more closely at merge_tests.py 20, I think the immediate
issue Paul raised is a design question: should a missing item be
flagged as a tree conflict by merge, or just notify the user that
we skipped it? I tend toward the former. In the source code, a
comment suggests we simply update the missing items automatically,
then continue the merge.

But anyway, I see in libsvn_client/merge.c that our tree conflict
detection code for handling missing and obstructing items is
inconsistent in handling dirs and files.

   merge_dir_opened():
     unversioned or is-file or missing or unknown or deleted:
       -> "local delete, incoming edit upon merge"

   merge_file_opened():
     unversioned or is-dir or missing or unknown:
       -> "local missing, incoming edit upon merge"

Looks like we need to clean up that code! After we sort out
issues 3334 and 3209 for update/switch, I guess the next step
is to sketch a table of how we handle each of these cases, to
make sure the high-level design is implemented like we planned
in /notes/tree-conflicts/*.txt.

Steve

>
> Thanks
>
> Mark
>
>
>
> On Wed, Feb 4, 2009 at 3:06 PM, Stephen Butler <sbutler_at_elego.de> wrote:
>> Hi Paul,
>>
>> summary: still hacking on groups 1 & 2, 3 is done, 4 is still open
>> for discussion.
>>
>> Some details inline, for anyone interested.
>>
>> Steve
>>
>> Quoting Paul Burba <ptburba_at_gmail.com>:
>>
>>> On Thu, Jan 22, 2009 at 11:20 AM, Stephen Butler <sbutler_at_elego.de> wrote:
>>>>
>>>> Quoting Paul Burba <ptburba_at_gmail.com>:
>>>>
>>>>> While reviewing issue #3209 I went through all the XFailing tests in
>>>>> checkout_tests.py, update_tests.py, switch_tests.py, and
>>>>> merge_tests.py looking for tests that fail because their expectations
>>>>> were not adjusted to account for new tree conflict behavior.
>>
>> [...]
>>>>>
>>>>> =======
>>>>> GROUP 1
>>>>> =======
>>>>>
>>>>> checkout_tests.py 13 'co handles obstructing paths scheduled for add'
>>>>> update_tests.py 34 'update handles obstructing paths scheduled for
>>>>> add'
>>>>> switch_tests.py 21 'forced switch detects tree conflicts'
>>>>>
>>>>> These three tests all cover behavior added in r22257:
>>>>>
>>>>> Enable up/sw/co to tolerate obstructions scheduled
>>>>> for addition without history.
>>
>> [...]
>>>>
>>>> I'd prefer to
>>>> tolerate the local-add-without-history (i.e., not raise a tree
>>>> conflict). To do this, I'll make the add_file() callback act like
>>>> add_directory(), checking for the local-add before checking for a tree
>>>> conflict.
>>
>> I think I have a fix for this, but am running into OS X "bus error",
>> I think due to SQLite, so I can't run tests and commit yet. :-(
>>
>> [...]
>>>>>
>>>>> trunk>svn up checkout_tests-13
>>>>> ......subversionlibsvn_wcadm_files.c:672: (apr_err=155000)
>>>>> svn: Revision 3 doesn't match existing revision 1 in
>>>>> 'checkout_tests-13/A/D/M'
>>>>
>>>> You're right, the update editor is being too picky here. Calling
>>>> svn_wc_ensure_adm3() is overkill anyway, it includes code that creates
>>>> the admin dir if it doesn't exist. All we want to do here is a
>>>> read-only sanity check. I'll change it to compare only the incoming
>>>> and existing URLs. If they don't match, there will be an error.
>>
>> The fix is part of the fix mentioned above.
>>
>>>>
>>>>>
>>>>>
>>>>> =======
>>>>> GROUP 2
>>>>> =======
>>>>>
>>>>> update_tests.py 31 'forced up fails with some types of obstructions'
>>>>>
>>>>> This test fails when we a (--forced) update tries to add a directory
>>>>> when a versioned directory of the same name already exists.
>>
>> [...]
>>>>>
>>>>> Shouldn't the update simply tolerate A/C/I's presence and simply bring
>>>>> A/C up to r2? Though if A/C/I pointed to a different location then
>>>>> there should be an error, but what?
>>>>
>>>> I agree, it would be nice to subsume the existing A/C/I working copy
>>>> when checking out a new working copy in the parent dir, and simply to
>>>> bring A/C/I up to date, raising new conflicts within it as necessary.
>>>>
>>>> If the URLs don't match, I'll raise an error, because it's the same
>>>> situation as an existing added-without-history item (see end of "GROUP
>>>> 1", above).
>>>>
>>>>>
>>>>> FWIW this doesn't seem a very common use case. The old behavior was
>>>>> added by me back with the r20945 'takeover' functionality, and IIRC it
>>>>> was simply because this was a possibility and the code had to do
>>>>> *something* in this case.
>>
>> Not done yet.
>>
>>>>>
>>>>>
>>>>> =======
>>>>> GROUP 3
>>>>> =======
>>>>>
>>>>> update_tests.py 50 'tree conflicts on update 2.3'
>>>>>
>>>>> This tests that updates of tree conflicted paths report the TCs as
>>>>> skipped, but what exactly does 2.3 refer to?
>>>>
>>>> To no other document. The names 2.1 and 2.2 are arbitrary, too.
>>>> Descriptive docstrings might help.
>>
>> Better docstrings checked in on trunk as r35683. At least I'm able
>> to finish something today!
>>
>>>>>
>>>>> =======
>>>>> GROUP 4
>>>>> =======
>>>>>
>>>>> merge_tests.py 20 'merge into missing must not break working copy'
>>>>>
>>>>> In r31326 (on the tree-conflicts branch) this test was changed to
>>>>> expect a tree conflict when merging into a directory that has a
>>>>> missing subtree (to be clear, this is missing in the '!' removed by a
>>>>> non-svn command sense of missing). The current (and pre-TC?) behavior
>>>>> was to report the missing subtrees as skipped.
>>>>>
>>>>> Why is a tree conflict expected here? We report the files as skipped
>>>>> during the merge and they show as missing in status. You can't even
>>>>> commit the change without updating first to get the missing items
>>>>> back. Yes, at that point you could commit and most likely you won't
>>>>> have what you want in the repos, but you must willfully ignore a lot
>>>>> of warnings that something is not quite right.
>>>>>
>>>
>>>
>>> Hi Steve,
>>>
>>> Any progress on any of the aforementioned todos? If not, will you be
>>> able to work on any of them? We still have issue #3209 as a blocker
>>> for 1.6 so we need to figure out what, if anything really needs to be
>>> done before 1.6.
>>>
>>> Paul
>>>
>>
>>
>>
>> --
>> Stephen Butler | Software Developer
>> elego Software Solutions GmbH
>> Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
>> fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
>> fax: +49 30 2345 8695 | http://www.elegosoft.com
>> Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
>> Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
>>
>>
>>
>>
>
>
>
> --
> Thanks
>
> Mark Phippard
> http://markphip.blogspot.com/
>

-- 
Stephen Butler | Software Developer
elego Software Solutions GmbH
Gustav-Meyer-Allee 25 | 13355 Berlin | Germany
fon: +49 30 2345 8696 | mobile: +49 163 25 45 015
fax: +49 30 2345 8695 | http://www.elegosoft.com
Geschäftsführer: Olaf Wagner | Sitz der Gesellschaft: Berlin
Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Received on 2009-02-05 19:21:11 CET

This is an archived mail posted to the Subversion Dev mailing list.