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

Re: svn commit: r979303 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

From: Ramkumar Ramachandra <artagnon_at_gmail.com>
Date: Mon, 26 Jul 2010 22:17:44 +0530

Hi Bert,

rhuijben_at_apache.org writes:
> Author: rhuijben
> Date: Mon Jul 26 14:24:43 2010
> New Revision: 979303
>
> URL: http://svn.apache.org/viewvc?rev=979303&view=rev
> Log:
> Add a simple property verifyer to the upgrade tests to test if the upgrade
> code correctly handles property upgrades. This makes issue #2530 visible on
> the current test data.
>
> * subversion/tests/cmdline/upgrade_tests.py
> (simple_property_verify): New helper function.
> (do_x3_upgrade): Verify properties before and after revert ti
> show handling of revert properties.
>
> Modified:
> subversion/trunk/subversion/tests/cmdline/upgrade_tests.py

I'm reviewing this patch on your request. Hope you find it useful.

> + # Shows all items in dict1 that are not also in dict2

Um, Python has docstrings just for this:
def diff_props(...):
"""You docstring here"""

> + def diff_props(dict1, dict2, name, match):
> +
> + equal = True;
> + for key in dict1:
> + node = dict1[key]
> + node2 = dict2.get(key, None)
> + if node2:
> + for prop in node:
> + v1 = node[prop]
> + v2 = node2.get(prop, None)
> +
> + if not v2:
> + print('\'%s\' property on \'%s\' not found in %s' %
> + (prop, key, name))
> + equal = False
> + if match and v1 != v2:
> + print('Expected \'%s\' on \'%s\' to be \'%s\', but found \'%s\'' %
> + (prop, key, v1, v2))
> + equal = False
> + else:
> + print('\'%s\': %s not found in %s' % (key, dict1[key], name))
> + equal = False
> +
> + return equal

Here's a cleaner version of the same thing (WARNING: Untested; I might
have made some silly mistakes)

def diff_props(dict1, dict2, name, match):
    equal = True
    for (key, value1) in dict1.iteritems():
        if dict2.has_key(key):
            value2 = dict2[key]
            # Try to match props in value1 and value2
            for prop in value1:
                if not value2.contains(prop):
                    print("'%s' property on '%s' not found in %s" % (prop, key, name))
                elif match and (value1[prop] != value2[prop]):
                    print("Expected '%s' on '%s' to be '%s', but found '%s'" %
                          (prop, key, value1[prop], value2[prop]))
                    print("'%s' property on '%s' not found in %s" % (prop, key, name))
                    equal = False
        else:
            print("'%s': %s not found in %s" % (key, value1, name))
            equal = False

  return equal

> + exit_code, output, errput = svntest.main.run_svn(None, 'proplist', '-R',
> + '-v', dir_path)

If you're not using some/ all of them, you can just replace them with
an '_' like to tell Python that you don't care about them:

_, output, _ = function_that_returns_a_3_tuple()

> + for i in output:

Poor variable name. I'd choose something like `for line in
output`. Python should read like English :)

> + if i.startswith('Properties on '):
> + target = i[15+len(dir_path)+1:-3].replace(os.path.sep, '/')

`15+len(dir_path)+1`? This can easily be written more nicely with a
few intermediate variables.

> + elif not i.startswith(' '):
> + name = i.strip()
> + else:
> + v = actual_props.get(target, {})
> + v[name] = i.strip()
> + actual_props[target] = v

Can't you switch these two conditions? The main message you're tying
to convey is special handling for the case i.startswith(' ').

> + v1 = diff_props(expected_props, actual_props, 'actual', True)
> + v2 = diff_props(actual_props, expected_props, 'expected', False)
> +
> + print('Actual properties: %s' % actual_props)
> + raise svntest.Failure("Properties unequal")

Instead of printing what's unequal in separate lines immediately, I'd
actually stuff them into a list and print them neatly in the end-
it'll be easier to read when you get lots of errors, but I wouldn't
bother otherwise.

These keys weren't in dict2: <keyx> ...
These properties weren't in the dict2: <keyx>:<propx> ...
These properties were in both dictionaries, but their values didn't
match: <keyx>:<propx>:<valuex> ...

-- Ram
Received on 2010-07-26 18:50:11 CEST

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