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

Add new base-path parameter to enhance the component yml from test workflow #3497

Merged
merged 8 commits into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion src/test_workflow/integ_test/integ_test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def __init__(self, args: TestArgs, test_manifest: TestManifest, components: Comp

self.tests_dir = os.path.join(os.getcwd(), "test-results")
os.makedirs(self.tests_dir, exist_ok=True)
self.test_recorder = TestRecorder(self.args.test_run_id, "integ-test", self.tests_dir)
self.test_recorder = TestRecorder(self.args.test_run_id, "integ-test", self.tests_dir, args.base_path)

def run(self) -> TestSuiteResults:
with TemporaryDirectory(keep=self.args.keep, chdir=True) as work_dir:
Expand All @@ -45,6 +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]
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
6 changes: 6 additions & 0 deletions src/test_workflow/integ_test/integ_test_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class IntegTestSuite(abc.ABC):
build_manifest: BuildManifest
save_logs: LogRecorder
additional_cluster_config: dict
test_result_data: list

"""
Kicks off integration tests for a component based on test configurations provided in
Expand Down Expand Up @@ -55,6 +56,7 @@ def __init__(

self.save_logs = test_recorder.test_results_logs
self.additional_cluster_config = None
self.test_result_data = []

@abc.abstractmethod
def execute_tests(self) -> TestComponentResults:
Expand All @@ -80,6 +82,10 @@ def pretty_print_message(self, message: str) -> None:
def test_artifact_files(self) -> Dict[str, str]:
pass

@property
def result_data(self) -> list:
return self.test_result_data


class InvalidTestConfigError(Exception):
pass
17 changes: 9 additions & 8 deletions src/test_workflow/integ_test/integ_test_suite_opensearch.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +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)
test_result_data = TestResultData(
self.component.name,
test_config,
status,
stdout,
stderr,
self.test_artifact_files
self.test_result_data.append(
TestResultData(
self.component.name,
test_config,
status,
stdout,
stderr,
self.test_artifact_files
)
)
self.save_logs.save_test_result_data(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 @@ -108,15 +108,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)
test_result_data = TestResultData(
self.component.name,
test_config,
status,
stdout,
stderr,
self.test_artifact_files
self.test_result_data.append(
TestResultData(
self.component.name,
test_config,
status,
stdout,
stderr,
self.test_artifact_files
)
)
self.save_logs.save_test_result_data(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_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ def __init__(self) -> None:
parser = argparse.ArgumentParser(description="Test an OpenSearch Bundle")
parser.add_argument("test_manifest_path", type=TestArgsPathValidator.validate, help="Specify a test manifest path.")
parser.add_argument("-p", "--paths", nargs='*', action=TestKwargs, default={}, help="Specify paths for OpenSearch and OpenSearch Dashboards.")
# e.g. --base-path https://ci.opensearch.org/ci/dbc/integ-test/2.7.0/7771/linux/x64/tar/
parser.add_argument("--base-path", type=str, default="", help="Specify base paths for the integration test logs.")
parser.add_argument("--test-run-id", type=int, help="The unique execution id for the test")
parser.add_argument("--component", type=str, dest="components", nargs='*', help="Test a specific component or components instead of the entire distribution.")
parser.add_argument("--keep", dest="keep", action="store_true", help="Do not delete the working temporary directory.")
Expand All @@ -41,6 +43,7 @@ def __init__(self) -> None:
self.logging_level = args.logging_level
self.test_manifest_path = args.test_manifest_path
self.paths = args.paths
self.base_path = args.base_path


TestArgs.__test__ = False # type:ignore
36 changes: 30 additions & 6 deletions src/test_workflow/test_recorder/test_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import yaml

from paths.tree_walker import walk
from test_workflow.test_recorder.log_recorder import LogRecorder
from test_workflow.test_recorder.test_result_data import TestResultData

Expand All @@ -24,7 +25,7 @@ class TestRecorder:
remote_cluster_logs: Any
test_results_logs: Any

def __init__(self, test_run_id: str, test_type: str, tests_dir: str) -> None:
def __init__(self, test_run_id: str, test_type: str, tests_dir: str, base_path: str = None) -> None:
self.test_run_id = test_run_id
self.test_type = test_type
self.location = os.path.join(tests_dir, str(self.test_run_id), self.test_type)
Expand All @@ -33,6 +34,14 @@ def __init__(self, test_run_id: str, test_type: str, tests_dir: str) -> None:
self.local_cluster_logs = LocalClusterLogs(self)
self.remote_cluster_logs = RemoteClusterLogs(self)
self.test_results_logs = TestResultsLogs(self)
self.base_path = base_path

def _get_file_path(self, base_path: str, component_name: str, component_test_config: str) -> str:
if base_path.startswith("https://"):
file_path = "/".join([base_path.strip("/"), "test-results", str(self.test_run_id), self.test_type, component_name, component_test_config])
else:
file_path = self._create_base_folder_structure(component_name, component_test_config)
return file_path

def _create_base_folder_structure(self, component_name: str, component_test_config: str) -> str:
dest_directory = os.path.join(self.location, component_name, component_test_config)
Expand All @@ -46,17 +55,33 @@ def _generate_std_files(self, stdout: str, stderr: str, output_path: str) -> Non
stderr_file.write(stderr)

def _generate_yml(self, test_result_data: TestResultData, output_path: str) -> str:
base_file_path = self._get_file_path(self.base_path, test_result_data.component_name, test_result_data.component_test_config)

components_files = self._get_list_files(output_path)
test_result_file = self._update_absolute_file_paths(components_files, base_file_path, "")

outcome = {
"test_type": self.test_type,
"test_run_id": self.test_run_id,
"run_id": self.test_run_id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the rename?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want this key to be written above the test_result_files when generating the yaml file but not strong opinion to change it. Open to change it back.

"component_name": test_result_data.component_name,
"test_config": test_result_data.component_test_config,
"exit_code": test_result_data.exit_code,
"test_result": "PASS" if (test_result_data.exit_code == 0) else "FAIL",
"test_result_files": test_result_file
}
with open(os.path.join(output_path, "%s.yml" % test_result_data.component_name), "w") as file:
yaml.dump(outcome, file)
return os.path.realpath("%s.yml" % test_result_data.component_name)

def _update_absolute_file_paths(self, files: list, base_path: str, relative_path: str) -> list:
return [os.path.join(base_path, relative_path, file) for file in files]

# get a list of files within directory with relative paths.
def _get_list_files(self, dir: str) -> list:
files = []
for file_paths in walk(dir):
files.append(file_paths[1])
return files

def _copy_log_files(self, log_files: dict, dest_directory: str) -> None:
if log_files:
for log_dest_dir_name, source_log_dir in log_files.items():
Expand Down Expand Up @@ -106,15 +131,14 @@ def save_test_result_data(self, test_result_data: TestResultData) -> None:


class TestResultsLogs(LogRecorder):
__test__ = False # type:ignore
parent_class: TestRecorder

def __init__(self, parent_class: TestRecorder) -> None:
self.parent_class = parent_class

def save_test_result_data(self, test_result_data: TestResultData) -> None:
base = self.parent_class._create_base_folder_structure(test_result_data.component_name, test_result_data.component_test_config)
dest_directory = os.path.join(base, "test-results")
os.makedirs(dest_directory, exist_ok=False)
dest_directory = self.parent_class._create_base_folder_structure(test_result_data.component_name, test_result_data.component_test_config)
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,18 @@ def test_with_integ_test(self, mock_temp: Mock, mock_test_recorder: Mock, mock_s
mock_test_recorder_object = MagicMock()
mock_test_recorder.return_value = mock_test_recorder_object

mock_suite_object.result_data.__iter__.return_value = [MagicMock(), MagicMock()]

runner = IntegTestRunnerOpenSearch(self.args, self.test_manifest)

# call the test target
results = runner.run()

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_suite.assert_called_once_with(
mock_properties_object.dependency_installer,
mock_component,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,18 @@ def test_with_integ_test(self, mock_temp: Mock, mock_test_recorder: Mock, mock_s
mock_test_recorder_object = MagicMock()
mock_test_recorder.return_value = mock_test_recorder_object

mock_suite_object.result_data.__iter__.return_value = [MagicMock(), MagicMock()]

runner = IntegTestRunnerOpenSearchDashboards(self.args, self.test_manifest)

# call the test target
results = runner.run()

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_suite.assert_called_once_with(
mock_properties_dependency_object.dependency_installer,
mock_properties_object.dependency_installer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def test_execute_integtest_sh(self, mock_execute: Mock, mock_git: Mock, mock_tes
"cypress-report": os.path.join("dir", "test_working_directory", "cypress", "results")
}
)
self.save_logs.save_test_result_data.assert_called_once_with(mock_test_result_data_object)
assert(mock_test_result_data.return_value in suite.result_data)

@patch("os.path.exists")
@patch("test_workflow.integ_test.integ_test_suite_opensearch_dashboards.TestResultData")
Expand Down

This file was deleted.

Loading