> -----Original Message-----
> From: Philip Martin [mailto:philip@codematters.co.uk]
> Sent: woensdag 5 april 2006 23:34
> To: Lieven Govaerts
> Cc: julianfoad@btopenworld.com; dev@subversion.tigris.org
> Subject: Re: [PATCH] fix for issue 2475: ignore case for
> hostnames (repost)
>
> +====================================================================
> > +# Copyright (c) 2000-2006 CollabNet. All rights reserved.
>
> 2006 only?
Hmm, what do you mean?
> > +# General modules
> > +import os
> > +from urlparse import urlparse, urlunparse
>
> Are these needed?
Nope, leftovers from an earlier try, removed them.
> > + # Do a checkout, and verify the resulting output and
> disk contents.
> > + svntest.actions.run_and_verify_checkout(repo_url2,
> > + wc_dir2,
> > + expected_output,
> > + expected_wc)
>
> I've not done it in a python test, but it might be simpler
> and quicker to do an OS copy of sbox.wc_dir rather than a
> separate checkout.
That doesn't make any sense here, because our url(repo_url2) is different
(uppercase) from the one in our working copy.
> > +
> > + # Move a file from wc to wc2
> > + mu_path = os.path.join(sbox.wc_dir, 'A', 'mu') E_path =
> > + os.path.join(wc_dir2, 'A', 'B', 'E')
> > +
> > + svntest.main.run_svn(None, 'mv', mu_path, E_path)
>
> run_svn on it's own isn't much of a test, add
> run_and_verify_status calls to check what has happend.
Yup, added them.
> > +
> > + # Move a folder from wc to wc2
> > + E_path = os.path.join(sbox.wc_dir, 'A', 'B', 'E') A_path =
> > + os.path.join(wc_dir2, 'A')
> > +
> > + svntest.main.run_svn(None, 'mv', E_path, A_path)
>
> So "move between working copies" works, I didn't know that!
> It's not clear to me whether this was intentional or
> accidental. This also needs run_and_verify_status, maybe
> even run_and_verify_commit.
I added run_and_verify_status. Since directories are completely
self-contained I don't see a reason for this not to work!
> > +
> > + # Copy a folder from wc to wc2
> > + E_path = os.path.join(sbox.wc_dir, 'A', 'B', 'E') A_path =
> > + os.path.join(wc_dir2, 'A')
> > +
> > + svntest.main.run_svn(None, 'cp', E_path, A_path)
>
> Rather than duplicate the whole test, I'd just put the copies
> into the previous test along with the moves.
Hm, I tend to prefer more but smaller unit tests, but with the overlap here
it makes sense to combine them.
> > +
> > +# regression test for issue #2475 - direct copy in the
> repository #
> > +this test handles the 'direct move' case too, that uses
> the same code.
> > +def path_copy_in_repo(sbox):
> > + "case issue #2475 - direct copy in the repository"
> > + sbox.build()
> > +
> > + repo_url2 =
> change_case_of_hostname(svntest.main.current_repo_url)
> > +
> > + # Copy a file from repo to repo2
> > + mu_url = svntest.main.current_repo_url + '/A/mu'
> > + E_url = repo_url2 + '/A/B/E'
> > +
> > + svntest.main.run_svn(None, 'cp', mu_url, E_url, '-m',
> 'copy mu to
> > + /A/B/E')
>
> Again run_svn isn't much of a test, how about a run_and_verify_update?
Added that.
> > +
> > +# regression test for issue #2475 - checkout in a working copy def
> > +path_checkout(sbox):
> > + "case issue #2475 - checkout in wc"
> > + sbox.build()
> > +
> > + # checkout a working copy non-recursively
> > + wc_dir2 = sbox.wc_dir + '2'
> > +
> > + output, errput = svntest.main.run_svn (None, 'co',
> > + '--username',
> svntest.main.wc_author,
> > + '--password',
> svntest.main.wc_passwd,
> > + '-N',
> > + svntest.main.current_repo_url,
> > + wc_dir2)
>
> Why is this using a separate, non-recursive, working copy?
> Your comment tells me what you are doing, something that is
> obvious from the code, but it doesn't tell me why you are doing it.
I've removed this test altogether.
> > +
> > + # checkout in the same working copy, use repository url with
> > + different case
> > + repo_url2 =
> change_case_of_hostname(svntest.main.current_repo_url)
> > +
> > + # Generate the expected output tree.
> > + expected_output = svntest.main.greek_state.copy()
> > + expected_output.wc_dir = wc_dir2
> expected_output.tweak(status='A ',
> > + contents=None)
> > +
> > + # Generate an expected wc tree.
>
> You have put in lots of "trivial" comments like that one,
> they don't make the code easier to understand they just make
> it longer.
Mostly copy paste from other tests, removed them.
> > +
> > +### End of file.
> > \ No newline at end of file
>
> You need to add path_tests.py to build.conf to get it to run.
That was new for me, so I applied that in the attached patch, and used it to
fix a patch for issue 2486 which was committed last week.
Lieven.
[[[
Fix for issue #2475: Make sure scheme and hostname in URI's are handled in a
case insenitive manner.
Patch by: Lieven Govaerts <lgo@mobsol.be>
Found by: Andy Somerville
* subversion/libsvn_subr/path.c:
(svn_path_canonicalize): convert scheme and hostname to lowercase.
* subversion/libsvn_wc/entries.c:
(svn_wc__atts_to_entry): while reading from the entries file,
canonicalize url and repos.
* subversion/include/svn_path.h:
(svn_path_canonicalize): add comment
* subversion/tests/libsvn_subr/path-test.c:
(test_canonicalize): add test for uppercase hostnames
* subversion/libsvn_ra_dav/session.c:
(svn_ra_dav__open): cleaned up now obsolete tolower hack.
* subversion/tests/cmdline/path_tests.py:
new file, contains tests for issue #2475.
* build.conf:
added path_tests.py to list of python tests.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 8 00:44:16 2006