| Message ID | 20190920152804.12875-18-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Fix check_to_replace_node() | expand |
20.09.2019 18:27, Max Reitz wrote: > Signed-off-by: Max Reitz <mreitz@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. > + > + @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. > + > + 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]) (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) > + > + 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.. > + > + 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 > + > + 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] > + > + 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'([^\[]+)\[([^\]]+)\]') > >
On 26.09.19 16:07, Vladimir Sementsov-Ogievskiy wrote: > 20.09.2019 18:27, Max Reitz wrote: >> Signed-off-by: Max Reitz <mreitz@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'([^\[]+)\[([^\]]+)\]') >> >> > >
02.10.2019 15:40, Max Reitz wrote: > On 26.09.19 16:07, Vladimir Sementsov-Ogievskiy wrote: >> 20.09.2019 18:27, Max Reitz wrote: >>> Signed-off-by: Max Reitz <mreitz@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] brr, I just mistaken here: we are in same try-except as the following line, and we'll catch it of course >> >> >>> + >>> + 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'([^\[]+)\[([^\]]+)\]') >>> >>> >> >> > >
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" + + @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'] + + 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 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) + + node = next(node for node in graph['nodes'] \ + if node['id'] == node_id) + except StopIteration: + node = None + + assert node is not None or expected_node is None, \ + 'No node found under %s (but expected %s)' % \ + (path, expected_node) + + assert expected_node is not None or node is None, \ + 'Found node %s under %s (but expected none)' % \ + (node['name'], path) + + 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'([^\[]+)\[([^\]]+)\]')
Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/iotests.py | 48 +++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)