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

On Monday 14 March 2011 11:40 AM, Arwin Arni wrote:
> 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
I'm sorry I overlooked the docstring length again.. Here's the corrected
patch.

Regards,
Arwin Arni