On Sun, 2008-08-17 at 01:10 +0200, Neels Janosch Hofmeyr wrote:
> Currently, if a cmdline test fails at verifying the expected output-,
> disk-, status- or skip-tree, it shows the metadata of only the first
> encountered mismatching node.
>
> This patch prints out the complete actual tree with its metadata (in
> case of a tree node mismatch). It prints the tree in python syntax,
> which is easy to read and practical.
This sounds very good.
I didn't know what you meant by "in Python syntax" until...
> Instead of just being informed about the first encountered node
> mismatch, it is possible to see patterns in the different nodes'
> metadata at a quick glance.
>
> I can choose to copy-and-paste the error message's printout of the
> actually found tree into my python test script to pass the current
> situation.
... here. That's brilliant. I have always found the test scripts' output
a bit awkward to read, but especially I have always found it difficult
to write the code to set up the "expected" status tree structure.
> It's valuable information for the test developer, with relatively few
> additional output lines, in a familiar and practical format.
>
> For illustration, I simulated a failure of `update_tests.py 9'. Here is
> the complete error output; the 16 lines starting with "ACTUAL OUTPUT
> TREE:" are added by this patch:
>
>
> $ ./update_tests.py 9
> =============================================================
> Expected 'E' and actual 'E' in output tree are different!
> =============================================================
> EXPECTED NODE TO BE:
> =============================================================
> * Node name: E
> Path: svn-test-work/working_copies/update_tests-9/A/B/E
> Contents: N/A (node is a directory)
> Properties: {}
> Attributes: {'status': 'D '}
> Children: 2
> =============================================================
> ACTUAL NODE FOUND:
> =============================================================
> * Node name: E
> Path: svn-test-work/working_copies/update_tests-9/A/B/E
> Contents: N/A (node is a directory)
> Properties: {}
> Attributes: {'status': 'A '}
> Children: 2
> Unequal at node E
> Unequal at node B
> Unequal at node A
> Unequal at node update_tests-9
> Unequal at node working_copies
> Unequal at node svn-test-work
> ACTUAL OUTPUT TREE:
> svntest.wc.State('svn-test-work/working_copies/update_tests-9', {
> 'A' : Item(),
> 'A/mu' : Item(verb='Restored'),
> 'A/D' : Item(),
> 'A/D/G' : Item(),
> 'A/D/G/rho' : Item(verb='Restored'),
> 'A/D/H' : Item(status='A '),
> 'A/D/H/chi' : Item(status='A '),
> 'A/D/H/omega' : Item(status='A '),
> 'A/D/H/psi' : Item(status='A '),
> 'A/B' : Item(),
> 'A/B/E' : Item(status='A '),
> 'A/B/E/alpha' : Item(status='A '),
> 'A/B/E/beta' : Item(status='A '),
> })
We can imagine further enhancements such as markers indicating which of
these lines don't match, but this is already useful.
> [[[
> Make the cmdline tests print out the complete actual tree and meta-
> data, as python script, in case of a node mismatch in output, disk,
> status or skip tree.
>
> * subversion/tests/cmdline/svntest/tree.py
> (print_script): New function in SVNTreeNode class, prints out the node
> metadata as python script line.
> (dump_tree_script): New function, prints a whole tree as py script,
> using print_script().
>
> * subversion/tests/cmdline/svntest/actions.py
> (verify_update): Print the output or disk tree if tree comparison
> throws an exception, using dump_tree_script() from tree.py.
> (run_and_verify_merge2): Print the skip tree if tree comparison
> throws an exception, using dump_tree_script() from tree.py.
> (run_and_verify_status): Print the status tree if tree comparison
> throws an exception, using dump_tree_script() from tree.py.
It's good to avoid repetition by combining those lines like this:
* subversion/tests/cmdline/svntest/actions.py
(verify_update, run_and_verify_merge2, run_and_verify_status):
Print the output or disk tree if tree comparison throws an
exception, using dump_tree_script() from tree.py.
> + def print_script(self, stream = sys.stdout, strip = 0):
> + "Python-script-print the meta data for this node."
Please make the doc-string say what each of the arguments means. (Well,
'self' is probably an exception. The 'stream' argument is pretty obvious
but only because of its default value which might be changed in future,
so it should be mentioned.)
Suggestion: an introductory comment for this next block of 5 lines of
code, something like:
# Strip the root node name and the requested amount from 'path'.
> + path = self.path
> + if len(path) > len(root_node_name) and
> path.startswith(root_node_name):
> + path = path[(len(root_node_name)+1):]
> + if len(path) > strip:
> + path = path[strip:]
> + line = "%-20s: Item(" % ("'%s'" % path)
> + comma = False
> +
> + mime_type = self.props.get("svn:mime-type")
> + if not mime_type or mime_type.startswith("text/"):
> + if self.children is None and not self.contents is None:
I don't think the "self.children is None" check helps here, and it might
hide a bug by not showing anything if a node has both children AND
contents. How about just "if self.contents:"?
> + line += "contents=\"\"\"%s\"\"\"" % self.contents
> + comma = True
There should probable be an "else: line += 'content is binary data'".
> +
> + for name in self.atts:
> + if comma:
> + line += ", "
> + line += "%s='%s'" % (name, self.atts[name])
> + comma = True
> +
> + # Then, there are the props. I never needed to validate them in
> + # any of my tests. I havent checked how they would be scripted.
> Maybe
> + # like self.atts above?
> + #
> + # for name in self.props:
> + # if comma:
> + # line += ", "
> + # line += "%s='%s'" % (name, self.props[name])
> + # comma = True
Yes, I expect so, but I haven't investigated.
> + line += "),"
> + print >> stream, line
> +def dump_tree_script(n, strip=0):
> + "Print out a python script representation of the tree's structure."
The doc-string should say what the argument "n" is and what the argument
"strip" is.
> + if n.children is None:
> + tmp_children = []
> + else:
> + tmp_children = n.children
I think this construct is normally written in Python as:
tmp_children = n.children or []
because the "or" operator says, "if the first operand is false then use
the next operand".
> + if n.name != root_node_name:
> + if strip < len(n.path):
> + n.print_script(strip = strip)
I'm not sure about this block. It's not printing anything about the root
node? And it's not printing anything about nodes whose path is less than
or equal to the "strip" argument"?
> + for i in range(len(tmp_children)):
> + c = tmp_children[i]
> + if i == len(tmp_children
> + )-1:
> + dump_tree_script(c, strip=strip)
> + else:
> + dump_tree_script(c, strip=strip)
I guess you were experimenting with omitting the trailing comma on the
last item, or something.
I think the function body all reduces to:
[[[
# print this node
if n.name != root_node_name:
if strip < len(n.path):
n.print_script(strip = strip)
# print this node's children
for child in n.children or []:
dump_tree_script(child, strip=strip)
]]]
I will gladly commit your patch if you can tidy up or explain these
points.
Regards,
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-19 02:09:13 CEST