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

Re: Problem with svn log for unrelated revisions

From: Paul Burba <ptburba_at_gmail.com>
Date: Fri, 17 Jun 2011 11:09:57 -0400

On Fri, Jun 17, 2011 at 8:23 AM, Dirk Thomas <websvn_at_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_at_2
> svn log -r 9:2 path-to-wc/A/D/G/rho_at_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_at_2
> svn log -r HEAD:2 path-to-wc/A/D/G/rho_at_2
> actually show the log of the wrong resource - namely A/D/G/rho_at_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_at_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_at_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
Received on 2011-06-17 17:10:31 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.