-
Notifications
You must be signed in to change notification settings - Fork 283
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update integ test runner to record each configuration separately #4438
Changes from 4 commits
d7a8cab
b985877
33ccaba
61b7422
5be4e89
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,16 +114,16 @@ def custom_node_endpoint_encoder(node_endpoint: NodeEndpoint) -> dict: | |
self.repo_work_dir = os.path.join( | ||
self.repo.dir, self.test_config.working_directory) if self.test_config.working_directory is not None else self.repo.dir | ||
(status, stdout, stderr) = execute(cmd, self.repo_work_dir, True, False) | ||
self.test_result_data.append( | ||
TestResultData( | ||
self.component.name, | ||
test_config, | ||
status, | ||
stdout, | ||
stderr, | ||
self.test_artifact_files | ||
) | ||
test_result_data = TestResultData( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: name this local var as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the variable name. Please help take a look again. Thanks! |
||
self.component.name, | ||
test_config, | ||
status, | ||
stdout, | ||
stderr, | ||
self.test_artifact_files | ||
) | ||
self.save_logs.save_test_result_data(test_result_data) | ||
self.test_result_data.append(test_result_data) | ||
if stderr: | ||
logging.info("Stderr reported for component: " + self.component.name) | ||
logging.info(stderr) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,9 +35,10 @@ def setUp(self) -> None: | |
self.work_dir = Path("test_dir") | ||
|
||
@patch("os.path.exists", return_value=True) | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.IntegTestSuiteOpenSearch.multi_execute_integtest_sh") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason this and below tests updated is the behavior of python mock tests. |
||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.Topology") | ||
@patch("test_workflow.test_recorder.test_recorder.TestRecorder") | ||
def test_execute_with_multiple_test_configs(self, mock_test_recorder: Mock, mock_topology: Mock, *mock: Any) -> None: | ||
def test_execute_with_multiple_test_configs(self, mock_test_recorder: Mock, mock_topology: Mock, mock_multi_execute_integtest_sh: Mock, *mock: Any) -> None: | ||
test_config, component = self.__get_test_config_and_bundle_component("job-scheduler") | ||
dependency_installer = MagicMock() | ||
integ_test_suite = IntegTestSuiteOpenSearch( | ||
|
@@ -50,8 +51,6 @@ def test_execute_with_multiple_test_configs(self, mock_test_recorder: Mock, mock | |
mock_test_recorder | ||
) | ||
mock_topology.create().__enter__.return_value = [{"cluster_name": "cluster1", "data_nodes": [{"endpoint": "localhost", "port": 9200, "transport": 9300}], "cluster_manager_nodes": []}] | ||
mock_multi_execute_integtest_sh = MagicMock() | ||
IntegTestSuiteOpenSearch.multi_execute_integtest_sh = mock_multi_execute_integtest_sh # type: ignore | ||
mock_multi_execute_integtest_sh.return_value = "success" | ||
|
||
test_results = integ_test_suite.execute_tests() | ||
|
@@ -63,9 +62,10 @@ def test_execute_with_multiple_test_configs(self, mock_test_recorder: Mock, mock | |
call([{"cluster_name": "cluster1", "data_nodes": [{"endpoint": "localhost", "port": 9200, "transport": 9300}], "cluster_manager_nodes": []}], False, "without-security") | ||
]) | ||
|
||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.IntegTestSuiteOpenSearch.multi_execute_integtest_sh") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.Topology") | ||
@patch("test_workflow.test_recorder.test_recorder.TestRecorder") | ||
def test_execute_with_build_dependencies(self, mock_test_recorder: Mock, mock_topology: Mock, *mock: Any) -> None: | ||
def test_execute_with_build_dependencies(self, mock_test_recorder: Mock, mock_topology: Mock, mock_multi_execute_integtest_sh: Mock, *mock: Any) -> None: | ||
dependency_installer = MagicMock() | ||
test_config, component = self.__get_test_config_and_bundle_component("index-management") | ||
integ_test_suite = IntegTestSuiteOpenSearch( | ||
|
@@ -80,8 +80,6 @@ def test_execute_with_build_dependencies(self, mock_test_recorder: Mock, mock_to | |
|
||
mock_topology.create().__enter__.return_value = [{"cluster_name": "cluster1", "data_nodes": [{"endpoint": "localhost", "port": 9200, "transport": 9300}], "cluster_manager_nodes": []}] | ||
|
||
mock_multi_execute_integtest_sh = MagicMock() | ||
IntegTestSuiteOpenSearch.multi_execute_integtest_sh = mock_multi_execute_integtest_sh # type: ignore | ||
mock_multi_execute_integtest_sh.return_value = "success" | ||
|
||
integ_test_suite.execute_tests() | ||
|
@@ -93,6 +91,7 @@ def test_execute_with_build_dependencies(self, mock_test_recorder: Mock, mock_to | |
[{"cluster_name": "cluster1", "data_nodes": [{"endpoint": "localhost", "port": 9200, "transport": 9300}], "cluster_manager_nodes": []}], False, "without-security")] | ||
) | ||
|
||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.IntegTestSuiteOpenSearch.multi_execute_integtest_sh") | ||
@patch("test_workflow.test_recorder.test_recorder.TestRecorder") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.execute") | ||
def test_execute_without_build_dependencies(self, mock_execute: Mock, *mock: Any) -> None: | ||
|
@@ -160,9 +159,10 @@ def __get_test_config_and_bundle_component(self, component_name: str) -> Tuple[T | |
|
||
@patch("os.path.exists", return_value=True) | ||
@patch.object(ScriptFinder, "find_integ_test_script") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.IntegTestSuiteOpenSearch.multi_execute_integtest_sh") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.Topology") | ||
@patch("test_workflow.test_recorder.test_recorder.TestRecorder") | ||
def test_execute_with_working_directory(self, mock_test_recorder: Mock, mock_topology: Mock, mock_script_finder: Mock, *mock: Any) -> None: | ||
def test_execute_with_working_directory(self, mock_test_recorder: Mock, mock_topology: Mock, mock_multi_execute_integtest_sh: Mock, mock_script_finder: Mock, *mock: Any) -> None: | ||
test_config, component = self.__get_test_config_and_bundle_component("dashboards-reports") | ||
dependency_installer = MagicMock() | ||
integ_test_suite = IntegTestSuiteOpenSearch( | ||
|
@@ -178,8 +178,6 @@ def test_execute_with_working_directory(self, mock_test_recorder: Mock, mock_top | |
mock_topology.create().__enter__.return_value = [{"cluster_name": "cluster1", "data_nodes": [{"endpoint": "localhost", "port": 9200, "transport": 9300}], "cluster_manager_nodes": []}] | ||
mock_script_finder.return_value = "integtest.sh" | ||
|
||
mock_multi_execute_integtest_sh = MagicMock() | ||
IntegTestSuiteOpenSearch.multi_execute_integtest_sh = mock_multi_execute_integtest_sh # type: ignore | ||
mock_multi_execute_integtest_sh.return_value = "success" | ||
|
||
integ_test_suite.execute_tests() # type: ignore | ||
|
@@ -189,3 +187,60 @@ def test_execute_with_working_directory(self, mock_test_recorder: Mock, mock_top | |
True, | ||
"with-security" | ||
) | ||
|
||
@patch("os.path.exists") | ||
@patch("os.makedirs") | ||
@patch("test_workflow.test_recorder.test_recorder.TestRecorder") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.TestResultData") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.GitRepository.__checkout__") | ||
@patch("test_workflow.integ_test.integ_test_suite_opensearch.execute", return_value=True) | ||
def test_multi_execute_integtest_sh(self, mock_execute: Mock, mock_git: Mock, mock_test_result_data: Mock, | ||
mock_test_recorder: Mock, mock_makedirs: Mock, mock_path_exists: Mock, *mock: Any) -> None: | ||
mock_find = MagicMock() | ||
mock_find.return_value = "./integtest.sh" | ||
|
||
ScriptFinder.find_integ_test_script = mock_find # type: ignore | ||
|
||
mock_execute.return_value = ("test_status", "test_stdout", "") | ||
|
||
mock_test_result_data_object = MagicMock() | ||
mock_test_result_data.return_value = mock_test_result_data_object | ||
mock_path_exists.return_value = True | ||
|
||
test_config, component = self.__get_test_config_and_bundle_component("job-scheduler") | ||
dependency_installer = MagicMock() | ||
integ_test_suite = IntegTestSuiteOpenSearch( | ||
dependency_installer, | ||
component, | ||
test_config, | ||
self.bundle_manifest, | ||
self.build_manifest, | ||
self.work_dir, | ||
mock_test_recorder | ||
) | ||
|
||
self.assertEqual(integ_test_suite.repo.url, "https://github.com/opensearch-project/job-scheduler.git") | ||
self.assertEqual(integ_test_suite.repo.ref, "4504dabfc67dd5628c1451e91e9a1c3c4ca71525") | ||
integ_test_suite.repo.dir = "dir" | ||
|
||
# call the test target | ||
mock_endpoint = MagicMock() | ||
status = integ_test_suite.multi_execute_integtest_sh([mock_endpoint], True, "with-security") | ||
|
||
mock_find.assert_called() | ||
self.assertEqual(status, "test_status") | ||
mock_execute.assert_called() | ||
|
||
mock_test_result_data.assert_called_once_with( | ||
"job-scheduler", | ||
"with-security", | ||
"test_status", | ||
"test_stdout", | ||
"", | ||
{ | ||
"opensearch-integ-test": os.path.join("dir", "build", "reports", "tests", "integTest") | ||
} | ||
) | ||
|
||
assert(mock_test_result_data.return_value in integ_test_suite.result_data) | ||
self.assertEqual(integ_test_suite.additional_cluster_config, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: name this local var as
test_result_data_obj
ortest_result_data_local
to distinguish fromself.test_result_data
.