From d3900b771100e6369e5f9aaab3cd56b27c1a3ef8 Mon Sep 17 00:00:00 2001 From: noopur Date: Wed, 18 Dec 2024 10:36:34 +0000 Subject: [PATCH] Review comments incorporated Signed-off-by: noopur --- .github/actions/tr_post_test_run/action.yml | 9 +------- .github/actions/tr_pre_test_run/action.yml | 19 +---------------- .github/workflows/task_runner_basic_e2e.yml | 6 ++---- .../task_runner_dockerized_ws_e2e.yml | 8 +++---- .github/workflows/wf_functional_e2e.yml | 8 +++---- tests/end_to_end/models/aggregator.py | 10 ++++----- tests/end_to_end/models/collaborator.py | 18 ++++++++-------- tests/end_to_end/utils/federation_helper.py | 21 +++++++++---------- 8 files changed, 34 insertions(+), 65 deletions(-) diff --git a/.github/actions/tr_post_test_run/action.yml b/.github/actions/tr_post_test_run/action.yml index 797f28fa79..04444af4d9 100644 --- a/.github/actions/tr_post_test_run/action.yml +++ b/.github/actions/tr_post_test_run/action.yml @@ -1,13 +1,6 @@ --- -#--------------------------------------------------------------------------- # Composite Action to run post-test functions for task runner end to end tests -# Includes: -# - Print test summary -# - Create tar file -# - Upload artifacts -# -# Authors - Noopur, Payal Chaurasiya -#--------------------------------------------------------------------------- + name: 'Post-Test Functions' description: 'Run post-test functions' inputs: diff --git a/.github/actions/tr_pre_test_run/action.yml b/.github/actions/tr_pre_test_run/action.yml index 2c366bfab1..2949a5e8cc 100644 --- a/.github/actions/tr_pre_test_run/action.yml +++ b/.github/actions/tr_pre_test_run/action.yml @@ -1,12 +1,6 @@ --- -#--------------------------------------------------------------------------- # Composite Action to run pre-test functions for task runner end to end tests -# Includes: -# - Set up Python -# - Install dependencies -# -# Authors - Noopur, Payal Chaurasiya -#--------------------------------------------------------------------------- + name: 'Pre-Test Functions' description: 'Run pre-test functions' @@ -26,14 +20,3 @@ runs: pip install . pip install -r test-requirements.txt shell: bash - - - name: Create OpenFL image - if: ${{ github.workflow == 'Task_Runner_E2E_via_Docker' }} - id: create_openfl_image - run: | - echo "Creating openfl image with current repo and branch. This may take a few minutes..." - cd openfl-docker - docker build . -t openfl -f Dockerfile.base \ - --build-arg OPENFL_REVISION=https://github.com/${{ github.repository }}.git@${{ github.ref_name }} - echo "OpenFL image created successfully" - shell: bash diff --git a/.github/workflows/task_runner_basic_e2e.yml b/.github/workflows/task_runner_basic_e2e.yml index 69b67c3c82..c75deb5eb3 100644 --- a/.github/workflows/task_runner_basic_e2e.yml +++ b/.github/workflows/task_runner_basic_e2e.yml @@ -1,8 +1,6 @@ --- -#--------------------------------------------------------------------------- -# Workflow to run Task Runner end to end tests using bare metal approach -# Authors - Noopur, Payal Chaurasiya -#--------------------------------------------------------------------------- +# Task Runner E2E tests for bare metal approach + name: Task_Runner_E2E # Please do not modify the name as it is used in the composite action on: diff --git a/.github/workflows/task_runner_dockerized_ws_e2e.yml b/.github/workflows/task_runner_dockerized_ws_e2e.yml index a1e939b962..f48ae32471 100644 --- a/.github/workflows/task_runner_dockerized_ws_e2e.yml +++ b/.github/workflows/task_runner_dockerized_ws_e2e.yml @@ -1,9 +1,7 @@ --- -#--------------------------------------------------------------------------- -# Workflow to run Task Runner end to end tests using dockerized workspace -# Authors - Noopur, Payal Chaurasiya -#--------------------------------------------------------------------------- -name: Task_Runner_E2E_via_Dockerized_Workspace # Please do not modify the name as it is used in the composite action +# Task Runner E2E tests for dockerized approach + +name: Task_Runner_Dockerized_E2E # Please do not modify the name as it is used in the composite action on: workflow_dispatch: diff --git a/.github/workflows/wf_functional_e2e.yml b/.github/workflows/wf_functional_e2e.yml index f5deac471a..c51b7acc03 100644 --- a/.github/workflows/wf_functional_e2e.yml +++ b/.github/workflows/wf_functional_e2e.yml @@ -1,9 +1,7 @@ --- -#--------------------------------------------------------------------------- -# Workflow to run Task Runner E2E tests via Docker -# Authors - Noopur, Payal Chaurasiya -#--------------------------------------------------------------------------- -name: Workflow Functional E2E +# Workflow functional E2E tests + +name: Workflow_Functional_E2E on: pull_request: diff --git a/tests/end_to_end/models/aggregator.py b/tests/end_to_end/models/aggregator.py index 332b6a5558..795a15bed3 100644 --- a/tests/end_to_end/models/aggregator.py +++ b/tests/end_to_end/models/aggregator.py @@ -50,27 +50,27 @@ def generate_sign_request(self): except Exception as e: raise ex.CSRGenerationException(f"Failed to generate sign request for {self.name}: {e}") - def start(self, res_file, run_for_dockerized_ws=False): + def start(self, res_file, with_docker=False): """ Start the aggregator Args: res_file (str): Result file to track the logs - run_for_dockerized_ws (bool): Flag specific to dockerized workspace scenario. Default is False. + with_docker (bool): Flag specific to dockerized workspace scenario. Default is False. Returns: str: Path to the log file """ try: log.info(f"Starting {self.name}") - res_file = res_file if not run_for_dockerized_ws else os.path.basename(res_file) + res_file = res_file if not with_docker else os.path.basename(res_file) error_msg = "Failed to start the aggregator" fh.run_command( "fx aggregator start", error_msg=error_msg, container_id=self.container_id, - workspace_path=self.workspace_path if not run_for_dockerized_ws else "", + workspace_path=self.workspace_path if not with_docker else "", run_in_background=True, bg_file=res_file, - run_for_dockerized_ws=run_for_dockerized_ws + with_docker=with_docker ) log.info( f"Started {self.name} and tracking the logs in {res_file}." diff --git a/tests/end_to_end/models/collaborator.py b/tests/end_to_end/models/collaborator.py index b5474a5719..51cbfeba0a 100644 --- a/tests/end_to_end/models/collaborator.py +++ b/tests/end_to_end/models/collaborator.py @@ -84,12 +84,12 @@ def create_collaborator(self): log.error(f"{error_msg}: {e}") raise e - def import_pki(self, zip_name, run_for_dockerized_ws=False): + def import_pki(self, zip_name, with_docker=False): """ Import and certify the CSR for the collaborator Args: zip_name (str): Zip file name - run_for_dockerized_ws (bool): Flag specific to dockerized workspace scenario. Default is False. + with_docker (bool): Flag specific to dockerized workspace scenario. Default is False. Returns: bool: True if successful, else False """ @@ -101,8 +101,8 @@ def import_pki(self, zip_name, run_for_dockerized_ws=False): cmd, error_msg=error_msg, container_id=self.container_id, - workspace_path=self.workspace_path if not run_for_dockerized_ws else "", - run_for_dockerized_ws=run_for_dockerized_ws, + workspace_path=self.workspace_path if not with_docker else "", + with_docker=with_docker, ) fh.verify_cmd_output( output, return_code, error, error_msg, @@ -114,27 +114,27 @@ def import_pki(self, zip_name, run_for_dockerized_ws=False): raise e return True - def start(self, res_file, run_for_dockerized_ws=False): + def start(self, res_file, with_docker=False): """ Start the collaborator Args: res_file (str): Result file to track the logs - run_for_dockerized_ws (bool): Flag to run the collaborator inside a docker container + with_docker (bool): Flag to run the collaborator inside a docker container Returns: str: Path to the log file """ try: log.info(f"Starting {self.collaborator_name}") - res_file = res_file if not run_for_dockerized_ws else os.path.basename(res_file) + res_file = res_file if not with_docker else os.path.basename(res_file) error_msg = f"Failed to start {self.collaborator_name}" fh.run_command( f"fx collaborator start -n {self.collaborator_name}", error_msg=error_msg, container_id=self.container_id, - workspace_path=self.workspace_path if not run_for_dockerized_ws else "", + workspace_path=self.workspace_path if not with_docker else "", run_in_background=True, bg_file=res_file, - run_for_dockerized_ws=run_for_dockerized_ws + with_docker=with_docker ) log.info( f"Started {self.name} and tracking the logs in {res_file}." diff --git a/tests/end_to_end/utils/federation_helper.py b/tests/end_to_end/utils/federation_helper.py index bd94be8fa9..cbcc2a2d20 100644 --- a/tests/end_to_end/utils/federation_helper.py +++ b/tests/end_to_end/utils/federation_helper.py @@ -224,13 +224,13 @@ def copy_file_between_participants( return True -def run_federation(fed_obj, install_dependencies=True, run_for_dockerized_ws=False): +def run_federation(fed_obj, install_dependencies=True, with_docker=False): """ Start the federation Args: fed_obj (object): Federation fixture object install_dependencies (bool): Install dependencies on collaborators (default is True) - run_for_dockerized_ws (bool): Flag specific to dockerized workspace scenario. Default is False. + with_docker (bool): Flag specific to dockerized workspace scenario. Default is False. Returns: list: List of response files for all the participants """ @@ -245,7 +245,7 @@ def run_federation(fed_obj, install_dependencies=True, run_for_dockerized_ws=Fal constants.AGG_COL_RESULT_FILE.format( fed_obj.workspace_path, participant.name ), - run_for_dockerized_ws=run_for_dockerized_ws, + with_docker=with_docker, ) for participant in fed_obj.collaborators + [fed_obj.aggregator] ] @@ -276,7 +276,7 @@ def run_federation_for_dws(fed_obj, use_tls): workspace_path="", error_msg=f"Failed to extract certificates for {participant.name}", container_id=participant.container_id, - run_for_dockerized_ws=True, + with_docker=True, ) for participant in [fed_obj.aggregator] + fed_obj.collaborators ] @@ -293,7 +293,7 @@ def run_federation_for_dws(fed_obj, use_tls): executor.submit( collaborator.import_pki, zip_name=f"agg_to_col_{collaborator.name}_signed_cert.zip", - run_for_dockerized_ws=True, + with_docker=True, ) for collaborator in fed_obj.collaborators ] @@ -305,7 +305,7 @@ def run_federation_for_dws(fed_obj, use_tls): raise e # Start federation run for all the participants - return run_federation(fed_obj, run_for_dockerized_ws=True) + return run_federation(fed_obj, with_docker=True) def install_dependencies_on_collaborators(fed_obj): @@ -540,7 +540,7 @@ def run_command( run_in_background=False, bg_file=None, print_output=False, - run_for_dockerized_ws=False, + with_docker=False, ): """ Run the command @@ -551,14 +551,14 @@ def run_command( run_in_background (bool): Run the command in background bg_file (str): Background file (with path) print_output (bool): Print the output - run_for_dockerized_ws (bool): Flag specific to dockerized workspace scenario. Default is False. + with_docker (bool): Flag specific to dockerized workspace scenario. Default is False. Returns: tuple: Return code, output and error """ return_code, output, error = 0, None, None error_msg = error_msg or "Failed to run the command" - if run_for_dockerized_ws and container_id: + if with_docker and container_id: log.debug("Running command in docker container") if len(workspace_path): docker_command = f"docker exec -w {workspace_path} {container_id} sh -c " @@ -580,8 +580,7 @@ def run_command( if print_output: log.info(f"Running command: {command}") - log.debug("Running command on local machine") - if run_in_background and not run_for_dockerized_ws: + if run_in_background and not with_docker: bg_file = open(bg_file, "w", buffering=1) ssh.run_command_background( command,