diff --git a/tests/test_api_request_handler.py b/tests/test_api_request_handler.py index 0a678670..b63f95e3 100644 --- a/tests/test_api_request_handler.py +++ b/tests/test_api_request_handler.py @@ -1380,7 +1380,8 @@ def test_upload_attachments_413_error(self, api_request_handler: ApiRequestHandl request_id_to_result_id = {id(report_results[0]): 2001} # Call upload_attachments - api_request_handler.upload_attachments(report_results, request_id_to_result_id) + total_attachments = sum(len(r["attachments"]) for r in report_results) + api_request_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) # Verify the request was made (case-insensitive comparison) assert requests_mock.last_request.url.lower() == create_url("add_attachment_to_result/2001").lower() @@ -1400,7 +1401,8 @@ def test_upload_attachments_success(self, api_request_handler: ApiRequestHandler request_id_to_result_id = {id(report_results[0]): 2001} # Call upload_attachments - api_request_handler.upload_attachments(report_results, request_id_to_result_id) + total_attachments = sum(len(r["attachments"]) for r in report_results) + api_request_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) # Verify the request was made (case-insensitive comparison) assert requests_mock.last_request.url.lower() == create_url("add_attachment_to_result/2001").lower() @@ -1413,7 +1415,8 @@ def test_upload_attachments_file_not_found(self, api_request_handler: ApiRequest request_id_to_result_id = {id(report_results[0]): 2001} # Call upload_attachments - should not raise exception - api_request_handler.upload_attachments(report_results, request_id_to_result_id) + total_attachments = sum(len(r["attachments"]) for r in report_results) + api_request_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) @pytest.mark.api_handler def test_upload_attachments_empty_run_scenario( @@ -1446,7 +1449,8 @@ def test_upload_attachments_empty_run_scenario( request_id_to_result_id = {id(report_results[0]): 5001, id(report_results[1]): 5002} # Call upload_attachments - api_request_handler.upload_attachments(report_results, request_id_to_result_id) + total_attachments = sum(len(r["attachments"]) for r in report_results) + api_request_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) # Verify both attachments were uploaded correctly history = requests_mock.request_history @@ -1487,7 +1491,8 @@ def test_upload_attachments_duplicate_case_ids_different_results( } # Call upload_attachments - api_request_handler.upload_attachments(report_results, request_id_to_result_id) + total_attachments = sum(len(r["attachments"]) for r in report_results) + api_request_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) # Verify both attachments were uploaded correctly history = requests_mock.request_history diff --git a/tests_e2e/test_end2end.py b/tests_e2e/test_end2end.py index 6afe43e7..726af1e4 100644 --- a/tests_e2e/test_end2end.py +++ b/tests_e2e/test_end2end.py @@ -92,7 +92,7 @@ def test_cli_robot_report_RF50(self): [ "Processed 3 test cases in 2 sections.", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 3 test results in", ], ) @@ -113,7 +113,7 @@ def test_cli_robot_report_RF70(self): [ "Processed 3 test cases in 2 sections.", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 3 test results in", ], ) @@ -135,7 +135,7 @@ def test_cli_plan_id(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -158,7 +158,7 @@ def test_cli_plan_id_and_config_id(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -180,7 +180,7 @@ def test_cli_update_run_in_plan(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Updating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -202,7 +202,7 @@ def test_cli_update_run_in_plan_with_configs(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Updating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -223,7 +223,7 @@ def test_cli_matchers_auto(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -246,7 +246,7 @@ def test_cli_matchers_auto_update_run(self): [ "Processed 3 test cases in section [GENERIC-IDS-AUTO]", f"Updating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results in", ], ) @@ -290,7 +290,7 @@ def test_cli_matchers_name(self): "Processed 3 test cases in section [GENERIC-IDS-NAME]", "Found 3 test cases without case ID in the report file.", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 3 test results in", ], ) @@ -313,7 +313,7 @@ def test_cli_matchers_property(self): "Processed 3 test cases in section [GENERIC-IDS-PROP]", "Found 3 test cases without case ID in the report file.", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 3 test results in", ], ) @@ -334,7 +334,7 @@ def test_cli_attachments(self): [ "Processed 3 test cases in section [ATTACHMENTS]", f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 4 attachments for 2 test results.", + "Uploading 4/4 for 2 test results", "Submitted 3 test results in", ], ) @@ -376,7 +376,7 @@ def test_cli_multiple_case_ids(self): # Creates test run in TestRail f"Creating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", # Uploads attachments: Test 2 (1 × 3 cases) + Test 3 (1 × 2 cases) = 5 - "Uploading 5 attachments for 5 test results", + "Uploading 5/5 for 5 test results", # Submits results: 1 (single) + 3 (test 2) + 2 (test 3) = 6 total "Submitted 6 test results in", ], @@ -548,7 +548,7 @@ def test_cli_add_run_upload_results(self): output, [ f"Updating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results", ], ) @@ -755,7 +755,7 @@ def bug_test_cli_robot_description_bug(self): output, [ "Processed 3 test cases in 2 sections.", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 3 test results in", ], ) @@ -776,7 +776,7 @@ def bug_test_automation_id(self): output, [ f"Updating test run. Test run: {self.TR_INSTANCE}index.php?/runs/view", - "Uploading 1 attachments for 1 test results.", + "Uploading 1/1 for 1 test results", "Submitted 6 test results", ], ) diff --git a/trcli/api/api_request_handler.py b/trcli/api/api_request_handler.py index 524fd501..0f4db53b 100644 --- a/trcli/api/api_request_handler.py +++ b/trcli/api/api_request_handler.py @@ -286,13 +286,16 @@ def update_existing_case_references( ) -> Tuple[bool, str, List[str], List[str], List[str]]: return self.case_handler.update_existing_case_references(case_id, junit_refs, case_fields, strategy) - def upload_attachments(self, report_results: List[Dict], request_id_to_result_id: Dict[int, int]): - return self.result_handler.upload_attachments(report_results, request_id_to_result_id) + def upload_attachments( + self, report_results: List[Dict], request_id_to_result_id: Dict[int, int], total_attachments: int + ): + return self.result_handler.upload_attachments(report_results, request_id_to_result_id, total_attachments) def add_results(self, run_id: int) -> Tuple[List, str, int]: return self.result_handler.add_results(run_id) def handle_futures(self, futures, action_string, progress_bar) -> Tuple[list, str]: + responses_by_request = {} if action_string == "add_results" else None responses = [] error_message = "" try: @@ -300,10 +303,11 @@ def handle_futures(self, futures, action_string, progress_bar) -> Tuple[list, st arguments = futures[future] response = future.result() if not response.error_message: - responses.append(response) if action_string == "add_results": + responses_by_request[id(arguments)] = response progress_bar.update(len(arguments["results"])) else: + responses.append(response) if action_string == "add_case": arguments = arguments.to_dict() arguments.pop("case_id") @@ -323,6 +327,11 @@ def handle_futures(self, futures, action_string, progress_bar) -> Tuple[list, st except KeyboardInterrupt: self.__cancel_running_futures(futures, action_string) raise KeyboardInterrupt + + if action_string == "add_results" and responses_by_request: + request_bodies = list(futures.values()) + responses = [responses_by_request[id(req)] for req in request_bodies if id(req) in responses_by_request] + return responses, error_message def close_run(self, run_id: int) -> Tuple[dict, str]: diff --git a/trcli/api/result_handler.py b/trcli/api/result_handler.py index 56a39e33..fc80330c 100644 --- a/trcli/api/result_handler.py +++ b/trcli/api/result_handler.py @@ -8,6 +8,7 @@ """ import os +import threading from concurrent.futures import ThreadPoolExecutor, as_completed from beartype.typing import List, Tuple, Dict @@ -44,14 +45,59 @@ def __init__( self.__get_all_tests_in_run = get_all_tests_in_run_callback self.handle_futures = handle_futures_callback - def upload_attachments(self, report_results: List[Dict], request_id_to_result_id: Dict[int, int]): + def _upload_single_attachment(self, file_path: str, result_id: int, case_id: int) -> Tuple[bool, str]: """ - Upload attachments to test results. + Upload a single attachment file. + + :param file_path: Path to the attachment file + :param result_id: TestRail result ID to attach to + :param case_id: TestRail case ID (for error messages) + :return: Tuple of (success, error_message) + """ + try: + with open(file_path, "rb") as file: + response = self.client.send_post(f"add_attachment_to_result/{result_id}", files={"attachment": file}) + + # Check if upload was successful + if response.status_code != 200: + file_name = os.path.basename(file_path) + + # Handle 413 Request Entity Too Large specifically + if response.status_code == 413: + error_msg = FAULT_MAPPING["attachment_too_large"].format(file_name=file_name, case_id=case_id) + return False, f"{file_name} (case {case_id})" + else: + # Handle other HTTP errors + error_msg = FAULT_MAPPING["attachment_upload_failed"].format( + file_path=file_name, + case_id=case_id, + error_message=response.error_message or f"HTTP {response.status_code}", + ) + return False, f"{file_name} (case {case_id})" + return True, None + + except FileNotFoundError: + return False, f"{file_path} (case {case_id})" + except Exception as ex: + file_name = os.path.basename(file_path) if os.path.exists(file_path) else file_path + return False, f"{file_name} (case {case_id})" + + def upload_attachments( + self, report_results: List[Dict], request_id_to_result_id: Dict[int, int], total_attachments: int + ): + """ + Upload attachments to test results concurrently. :param report_results: List of test results with attachments from report :param request_id_to_result_id: Mapping from request object id to result_id + :param total_attachments: Total number of attachments to upload """ failed_uploads = [] + uploaded_count = 0 + count_lock = threading.Lock() + + # Prepare list of upload tasks + upload_tasks = [] for report_result in report_results: case_id = report_result["case_id"] # Use object identity to find the correct result_id for THIS specific result @@ -62,39 +108,49 @@ def upload_attachments(self, report_results: List[Dict], request_id_to_result_id continue for file_path in report_result.get("attachments"): + upload_tasks.append((file_path, result_id, case_id)) + + if not upload_tasks: + return + + # Use ThreadPoolExecutor for concurrent uploads + with ThreadPoolExecutor(max_workers=min(10, len(upload_tasks))) as executor: + # Submit all upload tasks + future_to_task = { + executor.submit(self._upload_single_attachment, file_path, result_id, case_id): (file_path, case_id) + for file_path, result_id, case_id in upload_tasks + } + + # Process completed uploads + for future in as_completed(future_to_task): + file_path, case_id = future_to_task[future] try: - with open(file_path, "rb") as file: - response = self.client.send_post( - f"add_attachment_to_result/{result_id}", files={"attachment": file} + success, error_msg = future.result() + + with count_lock: + if success: + uploaded_count += 1 + else: + if error_msg: + failed_uploads.append(error_msg) + # Log errors to stderr + file_name = os.path.basename(file_path) + self.environment.elog(f"Failed to upload attachment '{file_name}' for case {case_id}") + + # Update progress in place (overwrite the line) + self.environment.log( + f"\rUploading {uploaded_count}/{total_attachments} for {len(report_results)} test results.", + new_line=False, ) - # Check if upload was successful - if response.status_code != 200: - file_name = os.path.basename(file_path) - - # Handle 413 Request Entity Too Large specifically - if response.status_code == 413: - error_msg = FAULT_MAPPING["attachment_too_large"].format( - file_name=file_name, case_id=case_id - ) - self.environment.elog(error_msg) - failed_uploads.append(f"{file_name} (case {case_id})") - else: - # Handle other HTTP errors - error_msg = FAULT_MAPPING["attachment_upload_failed"].format( - file_path=file_name, - case_id=case_id, - error_message=response.error_message or f"HTTP {response.status_code}", - ) - self.environment.elog(error_msg) - failed_uploads.append(f"{file_name} (case {case_id})") - except FileNotFoundError: - self.environment.elog(f"Attachment file not found: {file_path} (case {case_id})") - failed_uploads.append(f"{file_path} (case {case_id})") except Exception as ex: - file_name = os.path.basename(file_path) if os.path.exists(file_path) else file_path - self.environment.elog(f"Error uploading attachment '{file_name}' for case {case_id}: {ex}") - failed_uploads.append(f"{file_name} (case {case_id})") + with count_lock: + file_name = os.path.basename(file_path) if os.path.exists(file_path) else file_path + self.environment.elog(f"Error uploading attachment '{file_name}' for case {case_id}: {ex}") + failed_uploads.append(f"{file_name} (case {case_id})") + + # Print newline after progress is complete + self.environment.log("") # Provide a summary if there were failed uploads if failed_uploads: @@ -156,10 +212,7 @@ def add_results(self, run_id: int) -> Tuple[List, str, int]: attachments_count = 0 for result in report_results_w_attachments: attachments_count += len(result["attachments"]) - self.environment.log( - f"Uploading {attachments_count} attachments " f"for {len(report_results_w_attachments)} test results." - ) - self.upload_attachments(report_results_w_attachments, request_id_to_result_id) + self.upload_attachments(report_results_w_attachments, request_id_to_result_id, attachments_count) else: self.environment.log(f"No attachments found to upload.")