Hi Bert,
[email protected] 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
