Kamesh Jayachandran writes:
> [[[
>
> Use time proven 'svn_path_is_ancestor' rather than custom incomplete code.
> Testcase to showcase the defect.
It is normal for a bug fix to be acompanied by a test case, so this last
line is unnecesary.
>
...
> Patch by: kameshj
> Found by: plundblad
^ I'm actually "lundblad", but I understand why you're confused;)
> Index: subversion/tests/cmdline/merge_tests.py
> ===================================================================
> --- subversion/tests/cmdline/merge_tests.py (revision 23505)
> +++ subversion/tests/cmdline/merge_tests.py (working copy)
...
> @@ -4563,6 +4639,161 @@
> finally:
> os.chdir(saved_cwd)
>
> +def call_set_path_only_on_subtree_while_doing_subtree_merges(sbox):
> + "call set_path only on subtree path"
This is not a good name and description for a test case, since it talks about
details of a specific implementation in the libs. The test
is still valid if we change the implementation to something completely
different. You should talk about what the test actually checks (that
path name components that are prefixes of other path name components
can cause the path with the longer component to be inappropriately
excluded). Coming up with a good terse name might be a challenge, but
letting implementation details leak out into the test suite might get very
confusing in the future.
Also, I'm not sure why you need this very deep tree structure for this,
but I haven't tried to reproduce it.
An alternative to adding a new test might be to introduce this into
existing tests that deal with excluding of subtrees. I don't know
if this is feasable, so this is just a suggestion.
Maybe that's better than having lots of tests caused by short-lived
bugs during the initial implementation.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Feb 27 17:45:07 2007