From: Rishabh Dave <ridave@redhat.com>
Date: Fri, 21 Jun 2019 10:13:07 +0000 (+0530)
Subject: test_volume_client: rewrite test_put_object_versioned
X-Git-Tag: v15.1.0~2171^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=37744ecc2b113c31d8f5f12ca13b6bdcc8e84306;p=ceph.git

test_volume_client: rewrite test_put_object_versioned

The test's success depends on whether CommandFailedError exception is
raised. This manner of testing is unreliable since there's no way to
check if the exception was raised by the statement (in the embedded
Python program) the test expects to.

This implies that this test's failure may go undetected. This is primary
reason why bugs like tracker #38946 occured despite of having a test.

Fixes: http://tracker.ceph.com/issues/39510
Signed-off-by: Rishabh Dave <ridave@redhat.com>
---

diff --git a/qa/tasks/cephfs/test_volume_client.py b/qa/tasks/cephfs/test_volume_client.py
index 9bb9342b2f1f..8558c9c755d5 100644
--- a/qa/tasks/cephfs/test_volume_client.py
+++ b/qa/tasks/cephfs/test_volume_client.py
@@ -36,6 +36,7 @@ class TestVolumeClient(CephFSTestCase):
 from __future__ import print_function
 from ceph_volume_client import CephFSVolumeClient, VolumePath
 from sys import version_info as sys_version_info
+from rados import OSError as rados_OSError
 import logging
 log = logging.getLogger("ceph_volume_client")
 log.addHandler(logging.StreamHandler())
@@ -976,24 +977,30 @@ vc.disconnect()
 
         # Test if put_object_versioned() crosschecks the version of the
         # given object. Being a negative test, an exception is expected.
-        with self.assertRaises(CommandFailedError):
-            self._volume_client_python(vc_mount, dedent("""
-                data, version = vc.get_object_and_version("{pool_name}", "{obj_name}")
+        expected_exception = 'rados_OSError'
+        output = self._volume_client_python(vc_mount, dedent("""
+            data, version = vc.get_object_and_version("{pool_name}", "{obj_name}")
 
-                if sys_version_info.major < 3:
-                    data = data + 'm1'
-                elif sys_version_info.major > 3:
-                    data = str.encode(data.decode('utf-8') + 'm1')
+            if sys_version_info.major < 3:
+                data = data + 'm1'
+            elif sys_version_info.major > 3:
+                data = str.encode(data.decode('utf-8') + 'm1')
 
-                vc.put_object("{pool_name}", "{obj_name}", data)
+            vc.put_object("{pool_name}", "{obj_name}", data)
 
-                if sys_version_info.major < 3:
-                    data = data + 'm2'
-                elif sys_version_info.major > 3:
-                    data = str.encode(data.decode('utf-8') + 'm2')
+            if sys_version_info.major < 3:
+                data = data + 'm2'
+            elif sys_version_info.major > 3:
+                data = str.encode(data.decode('utf-8') + 'm2')
 
+            try:
                 vc.put_object_versioned("{pool_name}", "{obj_name}", data, version)
-            """).format(pool_name=pool_name, obj_name=obj_name))
+            except {expected_exception}:
+                print('{expected_exception} raised')
+        """).format(pool_name=pool_name, obj_name=obj_name,
+                    expected_exception=expected_exception))
+        self.assertEqual(expected_exception + ' raised', output)
+
 
     def test_delete_object(self):
         vc_mount = self.mounts[1]