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.
>
> Ignoring tests that fail for reasons unrelated to TCs, those that are
> failing because they test unimplemented TC behavior, those that are
> actively being worked on by others, and those I was able to fix, there
> are six tests remaining that started failing with the advent of tree
> conflict handling. These fall into four groups:
Hi Paul,
thanks for this report! I wrote some comments and todo-items inline
below.
Regards,
Steve
>
> =======
> 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.
>
> It's no longer an error when an update, switch, or
> checkout attempts to add a path that is already
> scheduled for addition *without* history in the WC.
>
> In the case of dirs, the directory is tolerated and
> reported as 'E'xisting.
>
> For files, if the obstruction is identical textually
> to the addition from the repos it is reported as
> 'E'xisting, if it differs it's treated as a conflict.
>
> Property merges for both files and dirs are reported
> normally, i.e. ' ', 'C', 'G'.
>
> Obstructions scheduled for addition *with* history
> still result in an error.
>
> See these three threads for more on this change:
>
> http://svn.haxx.se/dev/archive-2006-08/0336.shtml
> http://svn.haxx.se/dev/archive-2006-09/0362.shtml
> http://svn.haxx.se/dev/archive-2006-10/0708.shtml
>
> All these tests start with two identical working copies, WC1 and WC2.
> A set of files and directories are added to the repository via WC1.
> Then the test adds the same set of paths *without* history to WC2.
> These local adds in WC2 fall into 3 categories:
>
> A) Files textually identical to the files added to WC1.
>
> Previously we expected these obstructions to be
> tolerated, reported as 'E'xisting, and leave no trace
> after the checkout. Now however this produces a tree
> conflict of the 'local add, incoming add upon update'
> type.
>
> B) Files which differ textually from the files added
> to WC1.
>
> Prior to TC we expected these to be conflicts and they
> still are, but they are tree conflicts of the 'local
> add, incoming add upon update' type instead.
>
> C) Added directories that have no properties, the same
> properties, or conflicting properties with the the
> directories added to WC1.
>
> Prior to TC we expected all of these to be tolerated and
> reported as 'E'xisting, though their properties might mer'G'e
> or 'C'onflict. That is still the case.
>
> The desired behavior in cases A) & B) need to be resolved. Do we want
> to support the behavior introduced in r22257 that tolerates
> obstructing adds with history or remove it? The behavior in case C)
> should follow the decision on A) and B), files and directories are
> treated consistently I think.
To see this from the user's point of view, let's refer to
notes/tree-conflicts/resolution.txt. In section "up/sw: Add onto
Add", there are 3 sample use cases. The local and incoming changes...
1. are the same patch (local change can be reverted, or merged).
2. are different, but share a name (local change can be renamed).
3. are similar, and should be merged.
1 and 3 are safe for reporting the existing paths and merging any
differences.
2 is a bit tricky to sort out, if the merge has already been
attempted by the update. Maybe the user should update back to the
old BASE revision, move the files, and add them again? I'll have to
try it myself.
Assuming that situation 2 isn't too tough for the user, 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.
>
> There is also a regression in these tests. Taking checkout_tests.py
> 13 for example, after the above cases are covered, the test does a
> URL-to-URL copy of A/D/G to A/D/M and then does a WC-to-WC copy of
> A/D/H
> to A/D/M, leaving us with (I'm simplifying here, the test does some
> other stuff
> as well):
>
> trunk>svn st checkout_tests-13 -u
> * checkout_tests-13\A\D\M\pi
> * checkout_tests-13\A\D\M\rho
> * checkout_tests-13\A\D\M\tau
> A + * - checkout_tests-13\A\D\M
> * 1 checkout_tests-13\A\D
> Status against revision: 3
>
> Then when we try to update (the test does a checkout but the effect
> is the same)
> we get this error:
>
> trunk>svn up checkout_tests-13
> ..\..\..\subversion\libsvn_wc\adm_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.
>
>
> =======
> 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.
> Admittedly this is quite a contrived situation: The test adds a
> directory A/C/I in r2, updates A/C to r1, then checks out %URL%/A/C/I
> directly to A/C/I in the WC. This leaves us with a situation where
> A/C thinks A/C/I is some unversioned item:
>
> >svn st update_tests-31.backup\A\C -v
> 1 1 jrandom update_tests-31.backup\A\C
> ? update_tests-31.backup\A\C\I
>
> >svn st update_tests-31.backup\A\C\I -v
> 2 2 jrandom update_tests-31.backup\A\C\I
>
> Prior to TC we expected this to fail with an error:
>
> svn: Failed to add directory 'update_tests-31.backup\A\C\I':
> a versioned directory of the same name already exists
>
> This is now a tree conflict:
>
> >svn up --force update_tests-31.backup
> C update_tests-31.backup\A\C\I
> At revision 2.
> Summary of conflicts:
> Tree conflicts: 1
>
> >svn st update_tests-31.backup
> ? C update_tests-31.backup\A\C\I
> > local add, incoming add upon update
>
> It strikes me that *both* of these behaviors are probably wrong.
> 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.
>
>
> =======
> 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.
tree conflicts 2.1 leaf edit, tree del on update
tree conflicts 2.2 leaf del, tree del on update
tree conflicts 2.3 skip TCs on 2nd update
>
> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/use-cases.txt
> has use case 2, but there are no subsections. This appears to be an
> test for issue #3329,
> http://subversion.tigris.org/issues/show_bug.cgi?id=3329#desc2, and as
> I noted there, many of the cases this test covers now work and report
> skips:
>
> a) Directory tree conflict victim is updated
>
> b) File tree conflict victim is updated
>
> c) Directory within a tree conflict victim is updated
>
> d) Fle within a tree conflict victim is updated
> (file doesn't exist in the repos)
>
> What doesn't work is:
>
> e) A file within a tree conflict victim is updated
> (file doesn't exist in the repos but shows as
> locally added)
>
> f) Directory update target is not tree conflicted
> but has tree conflicted file child.
>
> g) Directory update target is not tree conflicted
> but has tree conflicted dir child.
>
> In all three of these cases the update prints no skip notifications,
> the 'At revision N.' is the only output. The status of the WC is the
> same in each case, so no harm is done outside of the missing skip
> notifications.
>
> So, is this simply a test for issue #3329 or is there anything else to
> consider?
Yes, it's simply a regression test for #3329 "Update throws error when
skipping some tree conflicts".
>
>
> =======
> 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.
>
> This case mentioned in the 'SCENARIO PLAYGROUND' in
> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/scratch-pad.txt,
> but there is no conclusion:
>
> 'svn merge' pulls file modification atop missing file
>
> NOW: "Skipped missing target" message.
>
> NEW:
>
> There is also some mention of this in
> http://svn.collab.net/repos/svn/trunk/notes/tree-conflicts/design-overview.txt
>
> * Pre-existing obstructions (status "~") or missing items ("!") should be
> detected first, before attempting to apply the change, and abort the
> operation.
>
> What is the latest thinking on this scenario?
>
> ~~~~~
>
> Thoughts from TC savvy folks and anyone else are appreciated.
>
> Thanks,
>
> 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
Received on 2009-01-22 17:20:57 CET