Skip to content
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

Merged
merged 5 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/test_workflow/integ_test/integ_test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def run(self) -> TestSuiteResults:
if test_config.integ_test:
test_suite = self.__create_test_suite__(component, test_config, work_dir.path)
test_results = test_suite.execute_tests()
[self.test_recorder.test_results_logs.save_test_result_data(result_data) for result_data in test_suite.result_data]
[self.test_recorder.test_results_logs.generate_component_yml(result_data) for result_data in test_suite.result_data]
all_results.append(component.name, test_results)
else:
logging.info(f"Skipping integ-tests for {component.name}, as it is currently not supported")
Expand Down
18 changes: 9 additions & 9 deletions src/test_workflow/integ_test/integ_test_suite_opensearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,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(
Copy link
Member

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 or test_result_data_local to distinguish from self.test_result_data.

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Copy link
Member

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 or test_result_data_local to distinguish from self.test_result_data.

Copy link
Member Author

Choose a reason for hiding this comment

The 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)
Expand Down
3 changes: 3 additions & 0 deletions src/test_workflow/test_recorder/test_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ def save_test_result_data(self, test_result_data: TestResultData) -> None:
logging.info(f"Recording component test results for {test_result_data.component_name} at " f"{os.path.realpath(dest_directory)}")
self.parent_class._generate_std_files(test_result_data.stdout, test_result_data.stderr, dest_directory)
self.parent_class._copy_log_files(test_result_data.log_files, dest_directory)

def generate_component_yml(self, test_result_data: TestResultData) -> None:
dest_directory = self.parent_class._create_base_folder_structure(test_result_data.component_name, test_result_data.component_test_config)
self.parent_class._generate_yml(test_result_data, dest_directory)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_with_integ_test(self, mock_temp: Mock, mock_test_recorder: Mock, mock_s
self.assertEqual(results["sql"], mock_test_results)

mock_suite_object.result_data.__iter__.assert_called()
mock_test_recorder_object.test_results_logs.save_test_result_data.assert_called()
mock_test_recorder_object.test_results_logs.generate_component_yml.assert_called()

mock_suite.assert_called_once_with(
mock_properties_object.dependency_installer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def test_with_integ_test(self, mock_temp: Mock, mock_test_recorder: Mock, mock_s
self.assertEqual(results["sql"], mock_test_results)

mock_suite_object.result_data.__iter__.assert_called()
mock_test_recorder_object.test_results_logs.save_test_result_data.assert_called()
mock_test_recorder_object.test_results_logs.generate_component_yml.assert_called()

mock_suite.assert_called_once_with(
mock_properties_dependency_object.dependency_installer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Member Author

Choose a reason for hiding this comment

The 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.
We were mocking IntegTestSuiteOpenSearch.multi_execute_integtest_sh here so it will apply to all tests intended to run IntegTestSuiteOpenSearch.multi_execute_integtest_sh within this class. Patching this to enforce this mock only applied to this method.

@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(
Expand All @@ -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()
Expand All @@ -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(
Expand All @@ -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()
Expand All @@ -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:
Expand Down Expand Up @@ -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(
Expand All @@ -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
Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,7 @@ def test(self) -> None:
logs.save_test_result_data(mock_test_result_data)

mock_parent_class._copy_log_files.assert_called_once_with(mock_test_result_data.log_files, dest_directory)

logs.generate_component_yml(mock_test_result_data)

mock_parent_class._generate_yml.assert_called_once_with(mock_test_result_data, dest_directory)
Loading