Re: [PATCH] Add a test to cover the regression introduced in r1075802

On Monday 14 March 2011 02:47 AM, Daniel Becroft wrote:
> On Mon, Mar 14, 2011 at 7:06 AM, Daniel Becroft <djcbecroft_at_gmail.com
> <mailto:djcbecroft_at_gmail.com>> wrote:
>
> On Sat, Mar 12, 2011 at 1:50 AM, C. Michael Pilato
> <cmpilato_at_collab.net <mailto:cmpilato_at_collab.net>> wrote:
>
> On 03/11/2011 10:03 AM, Arwin Arni wrote:
> > Index: ../subversion/tests/cmdline/merge_tests.py
> >
> ===================================================================
> > --- ../subversion/tests/cmdline/merge_tests.py
> (revision 1080126)
> > +++ ../subversion/tests/cmdline/merge_tests.py
> (working copy)
> > @@ -16586,6 +16586,102 @@
> > if not os.access(beta_path, os.X_OK):
> > raise svntest.Failure("beta is not marked as executable
> after commit")
> >
> > +@XFail()
> > +def dry_run_merge_conflicting_binary(sbox):
> > + "dry run merge should not create conflict resolution files"
>
> This long description line triggers the AssertionError about
> the test
> docstring needing to be 50 characters or less.
>
> > + svntest.actions.run_and_verify_merge(other_wc, '2', '3',
> > + sbox.repo_url, None,
> > + expected_output,
> > +
> expected_mergeinfo_output,
> > + expected_elision_output,
> > + expected_disk,
> > + expected_status,
> > + expected_skip,
> > + None, None, None,
> None, None,
> > + True, True,
> '--allow-mixed-revisions',
> > + other_wc)
>
> As this is a test of a dry-run merge, I find the use of
> run_and_verify_merge() a bit obfuscating. I think it'd be
> better to
> explicitly run a --dry-run merge so that it's obvious that
> what you're
> testing is exactly that.
>
> And, as I said elsethread, the patch didn't even apply to
> HEAD. So that
> needs to be reworked.
>
>
> Hi Mike,
>
> One of the advantages in using run_and_verify_merge() is that if
> dry_run = TRUE, it does it's own check to ensure that the working
> copy is not modified. IMO, this is better than explicitly building
> the tree prior to the merge, and then re-checking the merge.
>
> However, I'm finding that running an explicit merge works, but
> running run_and_verify_merge() does not (conflict files still get
> created).
>
>
> Never mind, I just found the problem. Using run_and_verify_merge()
> with dry_run = True runs both a dry-run and a wet-run update. Since
> the wet-run update always gets run, the conflict files always get created.
>
So the test-case is fine right.. I mean.. The dry_run checks are
independent of the wet run.. They happen before the wet run happens..

Is there anything else that we need to add to this test?

I've attached an updated patch that should now apply without hiccups..

Regards,
Arwin Arni