On Fri, Jun 17, 2011 at 8:23 AM, Dirk Thomas <web...@dirk-thomas.net> wrote: > While working on issue 3830 i noticed the following behavior of svn log. > > In the test repository used in log_tests.py the path "A/D/G/rho" is: > - created in revision 1 > - deleted in revision 5 > - recreated in revision 8 > > The following commands result in the expected error "Unable to find > repository location for": > svn log -r 2:9 path-to-wc/A/D/G/rho@2 > svn log -r 9:2 path-to-wc/A/D/G/rho@2 > > But when using HEAD instead of revision 9 (which should be the same) the > commands: > svn log -r 2:HEAD path-to-wc/A/D/G/rho@2 > svn log -r HEAD:2 path-to-wc/A/D/G/rho@2 > actually show the log of the wrong resource - namely A/D/G/rho@8. > > This looks like bug to me. > I have added another test to log_tests.py which therefore currently fails. > > Can anybody comment on this and verify that this is a bug which should be > fixed?
Hi Dirk, That does appear to be a bug. I don't see a similar issue in the tracker; would you mind filing a new issue? No worries if you cannot, I can do it at some point today. I'll commit your patch once we have an issue. Note that this is not a regression from 1.6.17 so it probably may not be fixed for 1.7.0 (I'm not sure if this is hindering your work on issue #3830). A quick note on your test: > Index: tests/cmdline/log_tests.py > =================================================================== > --- tests/cmdline/log_tests.py (revision 1129130) > +++ tests/cmdline/log_tests.py (working copy) > @@ -1973,6 +1973,39 @@ > log_chain = parse_log_output(out) > check_merge_results(log_chain, expected_merges) > > +#---------------------------------------------------------------------- > +def log_unrelated(sbox): > + "'svn log -rM:N PEG', where M/N is unrelated to PEG" > + > + guarantee_repos_and_wc(sbox) > + > + unknown_location = ".*Unable to find repository location for.*" > + > + target = os.path.join(sbox.repo_url, 'A', 'D', 'G', 'rho') + "@2" Don't construct URLs with os.path.join, it uses os.sep to join the paths, which on windows is '\', so you won't end up with a valid URL. This is fine: target = sbox.repo_url + '/A/D/G/rho@2' > + # log for /A/D/G/rho, deleted in revision 5, recreated in revision 8 > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, unknown_location, > + 'log', '-r', '2:9', target) > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, unknown_location, > + 'log', '-r', '9:2', target) A few comments explaining where/why the test fails is always helpful. Sure, we'll have an associated issue and/or dev threads to check, but a quick bit like this never hurts: > + # Currently this test fails because instead of returning the expected > + # 'Unable to find repository location for ^/A/D/G/rho in revision 9' > + # error, the log for ^/A/D/G/rho@8 is returned, but that is an unrelated > + # line of history. This is especially helpful in more complex tests which start failing for other reasons. When someone comes along months (or even years) later to work on an issue it's really nice to know if you have new problems to tackle. > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, unknown_location, > + 'log', '-r', '2:HEAD', target) > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, unknown_location, > + 'log', '-r', 'HEAD:2', target) > + > + file_not_found = ".*File not found.*" > + > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, file_not_found, > + 'log', '-r', '6:7', target) > + exit_code, output, err = svntest.actions.run_and_verify_svn( > + None, None, file_not_found, > + 'log', '-r', '7:6', target) This is personal preference, but since this last block works maybe move it ahead of the failing block? Anytime we are testing something like this: Does A work? Yes! Does B work? No! Does C work? Yes! ... If B is the functionality that currently fails I like to put it at the end of the test (if possible). That way if C ever starts failing while we are working on a fix then we'll notice we broke something else. In a simple test like this it doesn't matter much, but in more complex tests one might get far along on a "solution" and then find it breaks something else (ask me how I know about this :-) Paul
