Fix review comments

Author: Junchao-Mellanox

tests/platform_tests/api/test_chassis_fans.py

@@ -233,5 +233,5 @@ def test_is_replaceable(self, platform_api_conn):
         for i in range(self.num_fans):
             replaceable = fan.is_replaceable(platform_api_conn, i)
             if self.expect(replaceable is not None, "Failed to perform is_replaceable for fan {}".format(i)):
-                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for fan [}".format(i))
+                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for fan {}".format(i))
         self.assert_expectations()

tests/platform_tests/api/test_fan_drawer.py

@@ -158,13 +158,13 @@ def test_set_fan_drawers_led(self, duthost, localhost, platform_api_conn):
     def test_get_position_in_parent(self, platform_api_conn):
         for i in range(self.num_fan_drawers):
             position = fan_drawer.get_position_in_parent(platform_api_conn, i)
-            self.expect(position is not None, "Failed to perform get_position_in_parent for fan drawer {}".format(i))
-            self.expect(isinstance(position, int), "Position value must be an integer value for fan drawer {}".format(i))
+            if self.expect(position is not None, "Failed to perform get_position_in_parent for fan drawer {}".format(i)):
+                self.expect(isinstance(position, int), "Position value must be an integer value for fan drawer {}".format(i))
         self.assert_expectations()
 
     def test_is_replaceable(self, platform_api_conn):
         for i in range(self.num_fan_drawers):
             replaceable = fan_drawer.is_replaceable(platform_api_conn, i)
-            self.expect(replaceable is not None, "Failed to perform is_replaceable for fan drawer {}".format(i))
-            self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for fan drawer [}".format(i))
+            if self.expect(replaceable is not None, "Failed to perform is_replaceable for fan drawer {}".format(i)):
+                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for fan drawer {}".format(i))
         self.assert_expectations()

tests/platform_tests/api/test_psu.py

@@ -211,22 +211,22 @@ def test_is_replaceable(self, platform_api_conn):
         for psu_id in range(self.num_psus):
             replaceable = psu.is_replaceable(platform_api_conn, psu_id)
             if self.expect(replaceable is not None, "Failed to perform is_replaceable for psu id {}".format(psu_id)):
-                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for psu id [}".format(psu_id))
+                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for psu id {}".format(psu_id))
         self.assert_expectations()
 
     def test_thermals(self, platform_api_conn):
         for psu_id in range(self.num_psus):
             try:
                 num_thermals = int(psu.get_num_thermals(platform_api_conn, psu_id))
             except Exception:
-                pytest.fail("num_thermals is not an integer")
+                pytest.fail("PSU {}: num_thermals is not an integer".format(psu_id))
 
             thermal_list = psu.get_all_thermals(platform_api_conn, psu_id)
-            pytest_assert(thermal_list is not None, "Failed to retrieve thermals")
-            pytest_assert(isinstance(thermal_list, list) and len(thermal_list) == num_thermals, "Thermals appear to be incorrect")
+            pytest_assert(thermal_list is not None, "Failed to retrieve thermals for psu {}".format(psu_id))
+            pytest_assert(isinstance(thermal_list, list) and len(thermal_list) == num_thermals, "Thermals appear to be incorrect for psu {}".format(psu_id))
 
             for i in range(num_thermals):
                 thermal = psu.get_thermal(platform_api_conn, psu_id, i)
-                self.expect(thermal and thermal == thermal_list[i], "Thermal {} is incorrect".format(i))
+                self.expect(thermal and thermal == thermal_list[i], "Thermal {} is incorrect for psu {}".format(i, psu_id))
         self.assert_expectations()
 

tests/platform_tests/api/test_sfp.py

@@ -432,24 +432,24 @@ def test_get_position_in_parent(self, platform_api_conn):
         self.assert_expectations()
 
     def test_is_replaceable(self, platform_api_conn):
-        for i in range(self.num_sfps):
-            replaceable = sfp.is_replaceable(platform_api_conn, i)
-            if self.expect(replaceable is not None, "Failed to perform is_replaceable for sfp {}".format(i)):
-                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for sfp [}".format(i))
+        for sfp_id in range(self.num_sfps):
+            replaceable = sfp.is_replaceable(platform_api_conn, sfp_id)
+            if self.expect(replaceable is not None, "Failed to perform is_replaceable for sfp {}".format(sfp_id)):
+                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for sfp {}".format(sfp_id))
         self.assert_expectations()
 
     def test_thermals(self, platform_api_conn):
-        for i in range(self.num_sfps):
+        for sfp_id in range(self.num_sfps):
             try:
-                num_thermals = int(sfp.get_num_thermals(platform_api_conn, i))
+                num_thermals = int(sfp.get_num_thermals(platform_api_conn, sfp_id))
             except Exception:
-                pytest.fail("num_thermals is not an integer")
+                pytest.fail("SFP {}: num_thermals is not an integer".format(sfp_id))
 
             thermal_list = sfp.get_all_thermals(platform_api_conn, i)
-            pytest_assert(thermal_list is not None, "Failed to retrieve thermals")
-            pytest_assert(isinstance(thermal_list, list) and len(thermal_list) == num_thermals, "Thermals appear to be incorrect")
+            pytest_assert(thermal_list is not None, "Failed to retrieve thermals for sfp {}".format(sfp_id))
+            pytest_assert(isinstance(thermal_list, list) and len(thermal_list) == num_thermals, "Thermals appear to be incorrect for sfp {}".format(sfp_id))
 
             for thermal_index in range(num_thermals):
                 thermal = sfp.get_thermal(platform_api_conn, i, thermal_index)
-                self.expect(thermal and thermal == thermal_list[i], "Thermal {} is incorrect".format(thermal_index))
+                self.expect(thermal and thermal == thermal_list[i], "Thermal {} is incorrect for sfp {}".format(thermal_index, sfp_id))
         self.assert_expectations()

tests/platform_tests/api/test_thermal.py

@@ -218,5 +218,5 @@ def test_is_replaceable(self, platform_api_conn):
         for i in range(self.num_thermals):
             replaceable = thermal.is_replaceable(platform_api_conn, i)
             if self.expect(replaceable is not None, "Failed to perform is_replaceable for thermal {}".format(i)):
-                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for thermal [}".format(i))
+                self.expect(isinstance(replaceable, bool), "Replaceable value must be a bool value for thermal {}".format(i))
         self.assert_expectations()