Re: [Qemu-devel] [PATCH v5 12/13] iotests: Add assert_json_filename_equal() method

[Max Reitz]

On 26.10.2016 12:41, Kevin Wolf wrote:
> Am 25.10.2016 um 15:11 hat Max Reitz geschrieben:
>> Since the order of keys in JSON filenames is not necessarily fixed, they
>> should not be compared to fixed strings. This method takes a Python dict
>> as a reference, parses a given JSON filename and compares both.
>>
>> Signed-off-by: Max Reitz <address@hidden>
>> ---
>>  tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index c589deb..1f30cfc 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -222,6 +222,19 @@ class QMPTestCase(unittest.TestCase):
>>                      self.fail('invalid index "%s" in path "%s" in "%s"' % 
>> (idx, path, str(d)))
>>          return d
>>  
>> +    def flatten_qmp_object(self, obj, output=None, basestr=''):
>> +        if output is None:
>> +            output = dict()
>> +        if isinstance(obj, list):
>> +            for i in range(len(obj)):
>> +                self.flatten_qmp_object(obj[i], output, basestr + str(i) + 
>> '.')
>> +        elif isinstance(obj, dict):
>> +            for key in obj:
>> +                self.flatten_qmp_object(obj[key], output, basestr + key + 
>> '.')
>> +        else:
>> +            output[basestr[:-1]] = obj # Strip trailing '.'
>> +        return output
>> +
>>      def assert_qmp_absent(self, d, path):
>>          try:
>>              result = self.dictpath(d, path)
>> @@ -252,6 +265,13 @@ class QMPTestCase(unittest.TestCase):
>>          self.assertTrue(False, "Cannot find %s %s in result:\n%s" % \
>>                  (node_name, file_name, result))
>>  
>> +    def assert_json_filename_equal(self, json_filename, reference):
>> +        '''Asserts that the given filename is a json: filename and that its
>> +           content is equal to the given reference object'''
>> +        self.assertEqual(json_filename[:5], 'json:')
>> +        
>> self.assertEqual(self.flatten_qmp_object(json.loads(json_filename[5:])),
>> +                         self.flatten_qmp_object(reference))
> 
> Why do we have to flatten the dicts instead of comparing them directly?

Well, I believe flattened and unflattened dicts to be equal when it
comes to JSON filenames, and I wanted to express this here. This is my
personal opinion, though, and I can see how others might disagree.

In any case, the block layer does not necessarily report JSON filenames
fully unflattened, at least not today. The test added in patch 13
compares these filenames against unflattened dicts, however, because
that was simpler to write. So there's a practical reason for flattening
them.

Maybe unflattening (or crumpling) the block layer options will be our
next great project. :-)

> Anyway, it seems to be correct:
> Reviewed-by: Kevin Wolf <address@hidden>

Thanks for reviewing,

Max

signature.asc
Description: OpenPGP digital signature