Uninstall OpenSearch after testing on dashboards and modify the exce…

…ption handle to skip irrelevant exception (#3823)

Signed-off-by: Zelin Hao <[email protected]>

src/system/process.py

@@ -66,9 +66,9 @@ def terminate(self) -> int:
                 try:
                     for item in proc.open_files():
                         if self.stdout.name == item.path:
-                            raise Exception(f"stdout {item} is being used by process {proc}")
-                except Exception as err:
-                    logging.error(f"{err.args}")
+                            logging.error(f"stdout {item} is being used by process {proc}")
+                except Exception:
+                    pass
             os.unlink(self.stdout.name)
             self.stdout = None
 
@@ -81,9 +81,9 @@ def terminate(self) -> int:
                 try:
                     for item in proc.open_files():
                         if self.stderr.name == item.path:
-                            raise Exception(f"stderr {item} is being used by process {proc}")
-                except Exception as err:
-                    logging.error(f"{err.args}")
+                            logging.error(f"stderr {item} is being used by process {proc}")
+                except Exception:
+                    pass
             os.unlink(self.stderr.name)
             self.stderr = None
 

src/test_workflow/test_cluster.py

@@ -83,18 +83,17 @@ def start(self) -> None:
     def terminate(self) -> None:
         if self.service:
             self.termination_result = self.service.terminate()
+            self.__save_test_result_data(self.termination_result)
+            self.service.uninstall()
 
         for service in self.dependencies:
             termination_result = service.terminate()
             self.__save_test_result_data(termination_result)
+            service.uninstall()
 
         if not self.termination_result:
             raise ClusterServiceNotInitializedException()
 
-        self.__save_test_result_data(self.termination_result)
-
-        self.service.uninstall()
-
     def __save_test_result_data(self, termination_result: ServiceTerminationResult) -> None:
         test_result_data = TestResultData(
             self.component_name,

tests/tests_system/test_process.py

@@ -5,6 +5,7 @@
 # this file be licensed under the Apache-2.0 license or a
 # compatible open source license.
 
+import logging
 import tempfile
 import unittest
 from unittest.mock import MagicMock, call, patch
@@ -61,13 +62,44 @@ def test_terminate_unstarted_process(self) -> None:
 
         self.assertEqual(str(ctx.exception), "Process has not started")
 
+    @patch('os.unlink')
     @patch('psutil.Process')
     @patch('subprocess.Popen')
     @patch('psutil.process_iter')
-    def test_terminate_file_not_closed(self, procs: MagicMock, subprocess: MagicMock, process: MagicMock) -> None:
+    def test_terminate_process_file_not_closed(self, mock_procs: MagicMock, mock_subprocess: MagicMock, mock_process: MagicMock, mock_os_unlink: MagicMock) -> None:
         process_handler = Process()
 
+        mock_process1 = MagicMock()
+        mock_process2 = MagicMock()
+
+        mock_item1 = MagicMock()
+        mock_process1.open_files.return_value = [mock_item1]
+
+        mock_item2 = MagicMock()
+        mock_process2.open_files.return_value = [mock_item2]
+
+        mock_procs.return_value = [mock_process1, mock_process2]
+
         process_handler.start("mock_command", "mock_cwd")
+
+        mock_item1.path = process_handler.stdout.name
+        mock_item2.path = process_handler.stderr.name
+
         process_handler.terminate()
 
-        procs.assert_called
+        mock_procs.assert_called()
+
+        mock_process1.open_files.assert_called()
+        mock_process2.open_files.assert_called()
+
+        with self.assertLogs(level='ERROR') as log:
+            logging.error(f'stdout {mock_item1} is being used by process {mock_process1}')
+            self.assertEqual(len(log.output), 1)
+            self.assertEqual(len(log.records), 1)
+            self.assertIn(f'stdout {mock_item1} is being used by process {mock_process1}', log.output[0])
+
+        with self.assertLogs(level='ERROR') as log:
+            logging.error(f'stderr {mock_item2} is being used by process {mock_process2}')
+            self.assertEqual(len(log.output), 1)
+            self.assertEqual(len(log.records), 1)
+            self.assertIn(f'stderr {mock_item2} is being used by process {mock_process2}', log.output[0])

tests/tests_test_workflow/test_integ_workflow/integ_test/test_local_test_cluster_opensearch_dashboards.py

@@ -98,7 +98,8 @@ def test_start(self, mock_service_opensearch_dashboards: Mock, mock_service_open
     @patch("test_workflow.integ_test.local_test_cluster_opensearch_dashboards.ServiceOpenSearch")
     @patch("test_workflow.integ_test.local_test_cluster_opensearch_dashboards.ServiceOpenSearchDashboards")
     @patch("test_workflow.test_cluster.TestResultData")
-    def test_terminate(self, mock_test_result_data: Mock, mock_service_opensearch_dashboards: Mock, mock_service_opensearch: Mock) -> None:
+    def test_terminate(self, mock_test_result_data: Mock, mock_service_opensearch_dashboards: Mock,
+                       mock_service_opensearch: Mock) -> None:
         mock_test_recorder = MagicMock()
         mock_local_cluster_logs = MagicMock()
         mock_test_recorder.local_cluster_logs = mock_local_cluster_logs
@@ -139,27 +140,29 @@ def test_terminate(self, mock_test_result_data: Mock, mock_service_opensearch_da
         mock_service_opensearch_dashboards_object.terminate.assert_called_once()
 
         mock_test_result_data.assert_has_calls([
+            call(self.component_name,
+                 self.component_test_config,
+                 123,
+                 "test stdout_data",
+                 "test stderr_data",
+                 mock_log_files
+                 ),
             call(
                 self.component_name,
                 self.component_test_config,
                 123,
                 "test stdout_data",
                 "test stderr_data",
                 mock_log_files_opensearch
-            ),
-            call(self.component_name,
-                 self.component_test_config,
-                 123,
-                 "test stdout_data",
-                 "test stderr_data",
-                 mock_log_files)
+            )
         ])
 
         self.assertEqual(mock_local_cluster_logs.save_test_result_data.call_count, 2)
 
     @patch("test_workflow.integ_test.local_test_cluster_opensearch_dashboards.ServiceOpenSearch")
     @patch("test_workflow.integ_test.local_test_cluster_opensearch_dashboards.ServiceOpenSearchDashboards")
-    def test_terminate_service_not_initialized(self, mock_service_opensearch_dashboards: Mock, mock_service_opensearch: Mock) -> None:
+    def test_terminate_service_not_initialized(self, mock_service_opensearch_dashboards: Mock,
+                                               mock_service_opensearch: Mock) -> None:
         mock_test_recorder = MagicMock()
         mock_local_cluster_logs = MagicMock()
         mock_test_recorder.local_cluster_logs = mock_local_cluster_logs