Re: svn commit: r25204 - trunk/subversion/tests/cmdline

On Wed, May 30, 2007 at 08:20:23AM -0700, kameshj@tigris.org wrote:
> +def copy_src_detection_bug_if_target_has_many_ancestors_in_same_commit(sbox):

The 80-column line length limit is a limit, not a target :-)

> + "incorrect copy source detection"
> +
> + # Incorrect copy source detection if many ancestor of target exists
> + # in the same commit as that of copy.

This should be a complete sentence. I assume it should say something
like "This tests that copy source detection is correct in the case where
many ancestors of a target exist in the same commit as a copy.", but
that's just a guess (and it's still unclear: a copy of what?).

> + svntest.actions.run_and_verify_commit(wc_dir, expected_output,
> + expected_status, None,
> + None, None, None, None, wc_dir)
> + saved_cwd = os.getcwd()
> + os.chdir(A_copy_of_B_C_path)

chdir() calls should be inside a try/finally block so that an exception
inside won't break the rest of the tests (due to failure to reset the
cwd).

> + svntest.actions.run_and_verify_svn(None, [], [], 'merge', '-g')
> + os.chdir(saved_cwd)

You don't actually appear to be testing that the merge is actually doing
anything? The comments above implied that the merge would fail to
detect the right copy source - can you check that?

> @@ -6206,6 +6253,7 @@
> XFail(merge_to_path_with_switched_children),
> merge_with_implicit_target_file,
> XFail(empty_rev_range_mergeinfo),
> + XFail(copy_src_detection_bug_if_target_has_many_ancestors_in_same_commit),

Please reduce the function name length so that this fits on one line.

Regards,
Malcolm