[v5,12/13] iotests: Add assert_json_filename_equal() method
diff mbox

Message ID 20161025131141.24762-13-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Oct. 25, 2016, 1:11 p.m. UTC
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 <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Kevin Wolf Oct. 26, 2016, 10:41 a.m. UTC | #1
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 <mreitz@redhat.com>
> ---
>  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?

Anyway, it seems to be correct:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Max Reitz Oct. 26, 2016, 2:24 p.m. UTC | #2
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 <mreitz@redhat.com>
>> ---
>>  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 <kwolf@redhat.com>

Thanks for reviewing,

Max

Patch
diff mbox

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))
+
     def cancel_and_wait(self, drive='drive0', force=False, resume=False):
         '''Cancel a block job and wait for it to finish, returning the event'''
         result = self.vm.qmp('block-job-cancel', device=drive, force=force)