Whoops, somehow I missed this review!
On 6/1/07, Paul Burba <pburba@collab.net> wrote:
> Hi Ben,
>
> Two questions on this patch:
>
> 1) The utility main.merge_notify_line() checks the new merge range
> notifications when merging to a file target. But we don't check the
> notifications when merging into a directory (i.e.
> actions.run_and_verify_merge/run_and_verify_merge2() simply doesn't take
> them into account). So the new code in do_merge() is never actually
> tested. Did you look into making run_and_verify_merge() handle this?
>
No, I didn't look into that. My only goal was to get the tests to
pass. The original design of the tests was that all 'actual output'
would be processed by a strict regular expression, and non-matching
lines would be ignored. That's currently the case for
run_and_verify_merge(); it simply ignores all the new "---" lines. I
was originally attempting to make our single-file-target merges get
processed by run_and_verify_merge() as well, but gave up on that.
Instead, run_and_verify_svn(merge) now has to explicitly expect the
"---" line. Not ideal, but at least we only have to change the "---"
expectation in one place.
If you want, we could spend more time on teaching the core regular
expressions (used by run_and_verify_merge()) to understand/expect the
"---" lines, but it would be tricky. Those lines don't really fit
into our "build a tree from output, then compare expected and actual
trees" philosophy. Where would they live in the tree structure?
In any case, the point is that our test system parses and compares
trees, not arbitrary output lines. For example,
run_and_verify_update() doesn't parse the "Updated to rX" line, but it
*does* notice whether the updated working copy has working revs of X
or not.
> >svn merge %URL%/A merge_tests-1\A_COPY -r3:5
> --- Merging revision 4:
> --- Merging revision 5:
> --- Merging revisions 4-5:
>
> I suppose this is better than what we did before, which was no output
> and a 'silent' change to the svn:mergeinfo props. At least it looks
> like something is happening now! So I suppose this is less of a
> question and just more of an observation.
>
Yeah, no big deal. No-ops are common. I don't see it as any
different than an update printing nothing but "Updated to rX".
Three possible options: (1) do nothing about this, (2) remove the
trailing colon, (3) show svn:mergeinfo changes as " M". :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 6 13:35:25 2007