Hey Steve! (sbutler)
That's so awesome! Somehow you've magically fixed almost everything on the
branch! All I had to do was raise and notify a tree-conflict on file
addition in a conflicted directory.
With that, one thing remains: There's still some differentiation needed
between raised and persisted tree-conflicts. But I guess we can treat this
separately (in need of more tests, see below).
Otherwise, the "only" thing that can lead update_tests.py to perfection is
the implementation of dirs_same_p() a.k.a. diff_summarize, to be able to see
whether a subtree we're deleting is really the same one.
...And discussion on how exactly the output should be.
E.g., I think it is nice like this:
A C alpha
D C beta
M C gamma
While, right now, update notifies more like this:
A C alpha
D C beta
C gamma
Should we add the M or lose the A and D? I'd say add the M and whatever else
would have been shown without a tree-conflict, to be able to see what was
tried by the update. (Maybe there are more cases to this, though.)
Either way, I think this branch is ready for reintegration into trunk!!
What do you think, Steve?
(still in need of a complete `make check' though)
Thanks again for the great improvements I found there today! I really didn't
know what to do about the test framework, it's great how that just works
now. The stuff I complained about before also makes sense to me, now.
Oh, I'd just like to note that there was a misconception in some comments in
update_tests.py:
[[[
- ### Current behavior (rather messy, isn't it?)
]]]
It wasn't reaaally that messy, it just missed the cases of file additions
into conflicted directories. ;)
[[[
# 1.1) local tree delete, incoming leaf edit
...
- ### Current behavior carries out deletions in conflicted trees.
- expected_state = state_after_leaf_edit.copy()
- expected_state.remove('F/alpha', 'DF/D1/beta', 'DDF/D1/D2/gamma')
+ # The directory structure is scheduled for delete but is still
+ # found on disk, because the way directory deletes are handled by svn.
+ # No files are present because alpha, beta, gamma were locally removed
+ # and delta, epsilon, zeta weren't added due to tree-conflicts in their
+ # parent directory.
+ expected_disk = state_after_leaf_edit.copy()
+ expected_disk.remove('F/alpha', 'DF/D1/beta', 'DDF/D1/D2/gamma',
+ 'D/D1/delta', 'DD/D1/D2/epsilon', 'DDD/D1/D2/D3/zeta')
]]]
The incoming action is a leaf edit, so no deletions are tried. The error was
that files were being added into a conflicted dir. (Why was this called
expected_state? It's called expected_disk everywhere else.)
[[[
- ### Current behavior records redundant conflicts!
+ ### Current behavior fails to show a status on files that would
+ ### have been added if there hadn't been a tree-conflict.
]]]
The complete local tree was removed, so if you try to add a file a few
levels inside a removed dir, all the non-existing path elements are in tree
conflict. Makes sense to me.
The check_tree_conflict() vs. svn_wc_conflicted_p() discussion made me
think: In the tests, I guess we should also try to run the same operation
that causes the conflicts again, to check that the persisting conflicts
aren't re-raised or missed altogether. Is this accounted for yet?
And to squeeze yet another topic into this mail: What's the deal with the
run_and_verify_unquiet_status tests -- is status not intended to show
tree-conflicts (on missing nodes) by default?
Thanks Steve, great stuff. Yesterday I was confused, but now it all makes sense.
~Neels
Received on 2008-10-25 06:50:05 CEST