[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

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

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Mon, 14 Mar 2011 19:58:10 +0530

On 03/14/2011 06:11 PM, Arwin Arni wrote:
> 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

Thanks Arwin. Committed your patch in r1081390.

With regards
Kamesh Jayachandran
Received on 2011-03-14 15:26:49 CET

This is an archived mail posted to the Subversion Dev mailing list.