On 26.09.19 16:07, Vladimir Sementsov-Ogievskiy wrote: > 20.09.2019 18:27, Max Reitz wrote: >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> tests/qemu-iotests/iotests.py | 48 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 48 insertions(+) >> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index daed4ee013..e6fb46287d 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -670,6 +670,54 @@ class VM(qtest.QEMUQtestMachine): >> >> return fields.items() <= ret.items() >> >> + ''' >> + @path is a string whose components are separated by slashes. >> + The first component is a node name, the rest are child names. >> + Examples: >> + - "qcow2-node/backing/file" >> + - "quorum-node/children.2/file" > > Possibly, separting node-name to first parameter and keeping child-path as > a second will simplify code a bit, and be more useful for cases when on caller > part node-name is in variable.
Sounds good. >> + >> + @expected_node may be None. >> + >> + @graph may be None or the result of an x-debug-query-block-graph >> + call that has already been performed. >> + ''' >> + def assert_block_path(self, path, expected_node, graph=None): >> + if graph is None: >> + graph = self.qmp('x-debug-query-block-graph')['return'] > > Yay! I'm happy to see that it's useful. :-) It’s probably the best query function we have. >> + >> + iter_path = iter(path.split('/')) >> + root = next(iter_path) >> + try: >> + node = next(node for node in graph['nodes'] if node['name'] == >> root) >> + except StopIteration: >> + node = None > > for such usage next has second optional argument: next(iterator[, default]) Great! > (don't think I teach you Python, actually you teach me, as before I didn't > know > correct way to search first element with condition) We learn from one another, which is the best case. >> + >> + for path_node in iter_path: >> + assert node is not None, 'Cannot follow path %s' % path >> + >> + try: >> + node_id = next(edge['child'] for edge in graph['edges'] \ >> + if edge['parent'] == >> node['id'] and >> + edge['name'] == path_node) > > Hmm here you allow default StopIteration exception [1] > > >> + >> + node = next(node for node in graph['nodes'] \ >> + if node['id'] == node_id) >> + except StopIteration: >> + node = None > > actually, I think this will never happen, so we may simplify code and allow > it to > throw StopIteration exception in this impossible case.. This is for a use case where the next child simply doesn’t exist, so you can do: assert_block_path('qcow2-node/backing', None) To verify that the qcow2 node has no backing file. >> + >> + assert node is not None or expected_node is None, \ >> + 'No node found under %s (but expected %s)' % \ >> + (path, expected_node) > > node may be None here only from last iteration, but it can't happen: if we > have edge > with child, we'll for sure have node with such node-name in graph node will always be set by the try-except block, won’t it? >> + >> + assert expected_node is not None or node is None, \ >> + 'Found node %s under %s (but expected none)' % \ >> + (node['name'], path) > > hmm, so expected_node=None means we want to prove that there is no such node? > It should > be mentioned in comment above the function. But this don't work due to [1] Hm, I seem to remember I tested all cases locally and they all worked. Max >> + >> + if node is not None and expected_node is not None: >> + assert node['name'] == expected_node, \ >> + 'Found node %s under %s (but expected %s)' % \ >> + (node['name'], path, expected_node) >> >> index_re = re.compile(r'([^\[]+)\[([^\]]+)\]') >> >> > >
signature.asc
Description: OpenPGP digital signature