From cf1b1070474a7106ed8bbd0a5b35028e12449ccf Mon Sep 17 00:00:00 2001 From: sujata-m Date: Mon, 14 Oct 2024 11:51:15 +0530 Subject: [PATCH 1/6] Upgrade package to 0.2.7 --- requirements.txt | 2 +- src/osw_validator.py | 5 +++-- src/validation.py | 14 ++++++++------ 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/requirements.txt b/requirements.txt index c8de9a2..4d6c8c6 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,4 @@ pydantic==1.10.4 python-ms-core==0.0.22 uvicorn==0.20.0 html_testRunner==1.2.1 -python-osw-validation==0.2.4 \ No newline at end of file +python-osw-validation==0.2.7 \ No newline at end of file diff --git a/src/osw_validator.py b/src/osw_validator.py index d5467b0..505e596 100644 --- a/src/osw_validator.py +++ b/src/osw_validator.py @@ -1,6 +1,5 @@ -import uuid +import gc import logging -import datetime import urllib.parse from typing import List from python_ms_core import Core @@ -89,6 +88,8 @@ def send_status(self, result: ValidationResult, upload_message: Upload): logger.info(f'Publishing message for : {upload_message.message_id}') except Exception as e: logger.error(f'Error occurred while publishing message for : {upload_message.message_id} with error: {e}') + finally: + gc.collect() def has_permission(self, roles: List[str], queue_message: Upload) -> bool: diff --git a/src/validation.py b/src/validation.py index dfc3e45..f69f7cb 100644 --- a/src/validation.py +++ b/src/validation.py @@ -1,3 +1,4 @@ +import gc import os import shutil import logging @@ -47,7 +48,7 @@ def is_osw_valid(self, max_errors) -> ValidationResult: else: result.validation_message = 'Failed to validate because unknown file format' logger.error(f' Failed to validate because unknown file format') - + gc.collect() return result # Downloads the single file into a unique directory @@ -56,15 +57,15 @@ def download_single_file(self, file_upload_path=None) -> str: unique_id = self.get_unique_id() if not is_exists: os.makedirs(DOWNLOAD_FILE_PATH) - unique_directory = os.path.join(DOWNLOAD_FILE_PATH,unique_id) + unique_directory = os.path.join(DOWNLOAD_FILE_PATH, unique_id) if not os.path.exists(unique_directory): os.makedirs(unique_directory) - + file = self.storage_client.get_file_from_url(self.container_name, file_upload_path) try: if file.file_path: file_path = os.path.basename(file.file_path) - local_download_path = os.path.join(unique_directory,file_path) + local_download_path = os.path.join(unique_directory, file_path) with open(local_download_path, 'wb') as blob: blob.write(file.get_stream()) logger.info(f' File downloaded to location: {local_download_path}') @@ -74,14 +75,14 @@ def download_single_file(self, file_upload_path=None) -> str: except Exception as e: traceback.print_exc() logger.error(e) + finally: + gc.collect() # Generates a unique string for directory def get_unique_id(self) -> str: unique_id = uuid.uuid1().hex[0:24] return unique_id - - @staticmethod def clean_up(path): if os.path.isfile(path): @@ -91,3 +92,4 @@ def clean_up(path): # folder = os.path.join(DOWNLOAD_FILE_PATH, path) logger.info(f' Removing Folder: {path}') shutil.rmtree(path, ignore_errors=False) + gc.collect() From 3a72530a0f97cd05b5815a47a7407b0aabc314e3 Mon Sep 17 00:00:00 2001 From: sujata-m Date: Mon, 14 Oct 2024 13:46:55 +0530 Subject: [PATCH 2/6] Fixe unit test cases --- .github/workflows/unit_tests.yaml | 71 ++++---- src/osw_validator.py | 2 +- .../unit_tests/test_queue_message_content.py | 4 + tests/unit_tests/test_service.py | 168 ++++++++++++++++++ 4 files changed, 204 insertions(+), 41 deletions(-) create mode 100644 tests/unit_tests/test_service.py diff --git a/.github/workflows/unit_tests.yaml b/.github/workflows/unit_tests.yaml index 92d59b8..8aed5b6 100644 --- a/.github/workflows/unit_tests.yaml +++ b/.github/workflows/unit_tests.yaml @@ -1,51 +1,42 @@ ---- name: Unit Tests - -############################# -# Start the job on all push # -############################# on: + workflow_dispatch: push: branches-ignore: - '**' - # Remove the line above to run when pushing to master pull_request: - branches: [master, dev, stage] + branches: [main, dev, stage] -############### -# Set the Job # -############### jobs: UnitTest: - name: Unit Test Cases - # Set the agent to run on runs-on: ubuntu-latest + + env: + DATABASE_NAME: test_database + steps: - - name: Checkout code - uses: actions/checkout@v3 - - - name: Set up Python - uses: actions/setup-python@v4 - with: - python-version: "3.10" # Use the appropriate Python version - - - name: Install dependencies - run: | - pip install -r requirements.txt - - - name: Run unit tests - run: | - python test_report.py - coverage run --source=src -m unittest discover -s tests/ - coverage report -m - exit_status=$? - - # Set the exit status as an output for later use - echo "::set-output name=exit_status::$exit_status" - - - name: Archive Coverage Report - if: ${{ always() }} # Upload the coverage report even if tests fail - uses: actions/upload-artifact@v2 - with: - name: htmlcov - path: htmlcov + - name: Checkout code + uses: actions/checkout@v2 + + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: '3.10' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r requirements.txt + + - name: Run tests with coverage + run: | + coverage run --source=src -m unittest discover -s tests/unit_tests + coverage xml + + - name: Check coverage + run: | + coverage report --fail-under=85 + + + + diff --git a/src/osw_validator.py b/src/osw_validator.py index 505e596..23c2aa0 100644 --- a/src/osw_validator.py +++ b/src/osw_validator.py @@ -57,7 +57,7 @@ def validate(self, received_message: Upload): if self.has_permission(roles=['tdei-admin', 'poc', 'osw_data_generator'], queue_message=received_message) is None: error_msg = 'Unauthorized request !' - logger.error(tdei_record_id, error_msg, received_message) + logger.error(f'{tdei_record_id}, {error_msg}, {received_message}') raise Exception(error_msg) file_upload_path = urllib.parse.unquote(received_message.data.file_upload_path) diff --git a/tests/unit_tests/test_queue_message_content.py b/tests/unit_tests/test_queue_message_content.py index 17422c2..904ad9e 100644 --- a/tests/unit_tests/test_queue_message_content.py +++ b/tests/unit_tests/test_queue_message_content.py @@ -19,6 +19,10 @@ def setUp(self): data = TEST_DATA self.upload = Upload(data) + def test_message(self): + self.upload.message = 'New message' + self.assertEqual(self.upload.message, 'New message') + def test_message_type(self): self.assertEqual(self.upload.message_type, 'workflow_identifier') self.upload.message_type = 'New messageType' diff --git a/tests/unit_tests/test_service.py b/tests/unit_tests/test_service.py new file mode 100644 index 0000000..408fca5 --- /dev/null +++ b/tests/unit_tests/test_service.py @@ -0,0 +1,168 @@ +import unittest +from unittest.mock import patch, MagicMock +from src.osw_validator import OSWValidator +from src.models.queue_message_content import ValidationResult + + +class TestOSWValidatorService(unittest.TestCase): + + @patch('src.osw_validator.Settings') + @patch('src.osw_validator.Core') + def setUp(self, mock_core, mock_settings): + # Mock Settings + mock_settings.return_value.event_bus.upload_subscription = 'test_subscription' + mock_settings.return_value.event_bus.upload_topic = 'test_request_topic' + mock_settings.return_value.event_bus.validation_topic = 'test_response_topic' + mock_settings.return_value.max_concurrent_messages = 10 + mock_settings.return_value.get_download_directory.return_value = '/tmp' + mock_settings.return_value.event_bus.container_name = 'test_container' + + # Mock Core + mock_core.return_value.get_topic.return_value = MagicMock() + mock_core.return_value.get_storage_client.return_value = MagicMock() + + # Initialize OSWValidator with mocked dependencies + self.service = OSWValidator() + self.service.storage_client = MagicMock() + self.service.container_name = 'test_container' + self.auth = MagicMock() + + # Define a sample message with proper strings + self.sample_message = { + 'messageId': 'c8c76e89f30944d2b2abd2491bd95337', # messageId as string + 'messageType': 'workflow_identifier', # Ensure this is a string + 'data': { + 'file_upload_path': 'https://tdeisamplestorage.blob.core.windows.net/tdei-storage-test/Archivew.zip', + 'user_id': 'c59d29b6-a063-4249-943f-d320d15ac9ab', + 'tdei_project_group_id': '0b41ebc5-350c-42d3-90af-3af4ad3628fb' + } + } + + @patch('src.osw_validator.QueueMessage') + @patch('src.osw_validator.Upload') + def test_subscribe_with_valid_message(self, mock_request_message, mock_queue_message): + # Arrange + mock_message = MagicMock() + mock_queue_message.to_dict.return_value = self.sample_message + mock_request_message.from_dict.return_value = mock_request_message + self.service.validate = MagicMock() + + # Act + self.service.start_listening() + callback = self.service.listening_topic.subscribe.call_args[1]['callback'] + callback(mock_message) + + # Assert + self.service.validate.assert_called_once_with(received_message=mock_request_message.data_from()) + + @patch('src.osw_validator.Validation') + def test_validate_with_valid_file_path(self, mock_validation): + # Arrange + mock_request_message = MagicMock() + mock_request_message.data.jobId = None + mock_request_message.data.file_upload_path = 'test_dataset_url' + + # Mock the Validation instance and its return value + mock_validation_instance = mock_validation.return_value + result = ValidationResult() + result.is_valid = True # Simulate successful validation + result.validation_message = '' + mock_validation_instance.validate.return_value = result + + self.service.send_status = MagicMock() + + # Act + self.service.validate(mock_request_message) + + # Assert + self.service.send_status.assert_called_once() + + # Extract the actual arguments used in the send_status call + call_args = self.service.send_status.call_args + actual_result = call_args[1]['result'] + actual_upload_message = call_args[1]['upload_message'] + + # Assert that the properties of the result match what we expect + self.assertEqual(actual_result.is_valid, result.is_valid) + self.assertEqual(actual_result.validation_message, result.validation_message) + + # Ensure the upload_message is the expected object + self.assertEqual(actual_upload_message, mock_request_message) + + @patch('src.osw_validator.ValidationResult') + def test_validate_with_no_file_upload_path(self, mock_validation_result): + # Arrange + mock_request_message = MagicMock() + mock_request_message.data.file_upload_path = None # Simulate None file upload path + + # Create the expected failure result + mock_result = mock_validation_result.return_value + mock_result.is_valid = False + mock_result.validation_message = 'Request does not have valid file path specified.' + + self.service.send_status = MagicMock() + + # Act + self.service.validate(mock_request_message) + + # Assert that send_status was called with failure result + self.service.send_status.assert_called_once() + + # Extract the actual arguments used in the send_status call + call_args = self.service.send_status.call_args + actual_result = call_args[1]['result'] + actual_upload_message = call_args[1]['upload_message'] + + # Assert that the result indicates failure + self.assertFalse(actual_result.is_valid) + self.assertEqual(actual_result.validation_message, 'Error occurred while validating OSW request Request does not have valid file path specified.') + + # Ensure the upload_message is the expected object + self.assertEqual(actual_upload_message, mock_request_message) + + @patch('src.osw_validator.Validation') + @patch('src.osw_validator.OSWValidator.has_permission') + def test_validate_with_validation_only_in_message_type(self, mock_has_permission, mock_validation): + """Test that has_permission is NOT called when 'VALIDATION_ONLY' is in message_type.""" + # Arrange + mock_request_message = MagicMock() + mock_request_message.message_id = 'message_id' + + # Properly mock the 'data' object to avoid AttributeError + mock_request_message.data = MagicMock() + mock_request_message.data.file_upload_path = 'test_dataset_url' + mock_request_message.message_type = 'VALIDATION_ONLY' + + # Ensure has_permission is not called + mock_has_permission.return_value = None + + # Mock the Validation instance to simulate a successful validation + mock_validation_instance = mock_validation.return_value + mock_validation_instance.validate.return_value.is_valid = True + mock_validation_instance.validate.return_value.validation_message = 'Validation successful' + + # Mock the send_status method + self.service.send_status = MagicMock() + + # Act + self.service.validate(mock_request_message) + + # Assert + # Ensure has_permission was NOT called since VALIDATION_ONLY is in the message_type + mock_has_permission.assert_not_called() + + # Ensure send_status was called with a successful validation result + self.service.send_status.assert_called_once() + + # Extract the actual result and upload message + actual_result = self.service.send_status.call_args[1]['result'] + actual_upload_message = self.service.send_status.call_args[1]['upload_message'] + + # Assert the result is valid and the upload_message is the mock_request_message + self.assertTrue(actual_result.is_valid) + self.assertEqual(actual_upload_message, mock_request_message) + + + +if __name__ == '__main__': + unittest.main() From 99c9abe53120680670aead15293a043e0583e3f9 Mon Sep 17 00:00:00 2001 From: sujata-m Date: Mon, 14 Oct 2024 14:28:48 +0530 Subject: [PATCH 3/6] Fixed pipeline and unit test cases --- src/validation.py | 48 ++- tests/unit_tests/test_validation.py | 517 +++++----------------------- 2 files changed, 124 insertions(+), 441 deletions(-) diff --git a/src/validation.py b/src/validation.py index f69f7cb..a4120d5 100644 --- a/src/validation.py +++ b/src/validation.py @@ -1,5 +1,6 @@ import gc import os +import time import shutil import logging import traceback @@ -11,7 +12,7 @@ ROOT_DIR = os.path.dirname(os.path.abspath(__file__)) # Path used for download file generation. -DOWNLOAD_FILE_PATH = f'{Path.cwd()}/downloads' +DOWNLOAD_DIR = f'{Path.cwd()}/downloads' logging.basicConfig() logger = logging.getLogger('OSW_VALIDATION') @@ -26,46 +27,55 @@ def __init__(self, file_path=None, storage_client=None): self.file_path = file_path self.file_relative_path = file_path.split('/')[-1] self.client = self.storage_client.get_container(container_name=self.container_name) + is_exists = os.path.exists(DOWNLOAD_DIR) + unique_id = self.get_unique_id() + if not is_exists: + os.makedirs(DOWNLOAD_DIR) + self.unique_dir_path = os.path.join(DOWNLOAD_DIR, unique_id) + if not os.path.exists(self.unique_dir_path): + os.makedirs(self.unique_dir_path) def validate(self, max_errors=20) -> ValidationResult: - return self.is_osw_valid(max_errors) + try: + return self.is_osw_valid(max_errors) + finally: + Validation.clean_up(self.unique_dir_path) def is_osw_valid(self, max_errors) -> ValidationResult: + start_time = time.time() result = ValidationResult() result.is_valid = False result.validation_message = '' root, ext = os.path.splitext(self.file_relative_path) if ext and ext.lower() == '.zip': downloaded_file_path = self.download_single_file(self.file_path) - logger.info(f' Downloaded file path: {downloaded_file_path}') - validator = OSWValidation(zipfile_path=downloaded_file_path) - validation_result = validator.validate(max_errors) - result.is_valid = validation_result.is_valid - if not result.is_valid: - result.validation_message = validation_result.errors - logger.error(f' Error While Validating File: {str(validation_result.errors)}') - Validation.clean_up(downloaded_file_path) + if downloaded_file_path: + logger.info(f' Downloaded file path: {downloaded_file_path}') + validator = OSWValidation(zipfile_path=downloaded_file_path) + validation_result = validator.validate(max_errors) + result.is_valid = validation_result.is_valid + if not result.is_valid: + result.validation_message = validation_result.errors + logger.error(f' Error While Validating File: {str(validation_result.errors)}') + Validation.clean_up(downloaded_file_path) + else: + result.validation_message = 'Failed to validate because unknown file format' else: result.validation_message = 'Failed to validate because unknown file format' logger.error(f' Failed to validate because unknown file format') + end_time = time.time() + time_taken = end_time - start_time + logger.info(f'Validation completed in {time_taken} seconds') gc.collect() return result # Downloads the single file into a unique directory def download_single_file(self, file_upload_path=None) -> str: - is_exists = os.path.exists(DOWNLOAD_FILE_PATH) - unique_id = self.get_unique_id() - if not is_exists: - os.makedirs(DOWNLOAD_FILE_PATH) - unique_directory = os.path.join(DOWNLOAD_FILE_PATH, unique_id) - if not os.path.exists(unique_directory): - os.makedirs(unique_directory) - file = self.storage_client.get_file_from_url(self.container_name, file_upload_path) try: if file.file_path: file_path = os.path.basename(file.file_path) - local_download_path = os.path.join(unique_directory, file_path) + local_download_path = os.path.join(self.unique_dir_path, file_path) with open(local_download_path, 'wb') as blob: blob.write(file.get_stream()) logger.info(f' File downloaded to location: {local_download_path}') diff --git a/tests/unit_tests/test_validation.py b/tests/unit_tests/test_validation.py index e70e872..b256d78 100644 --- a/tests/unit_tests/test_validation.py +++ b/tests/unit_tests/test_validation.py @@ -1,9 +1,8 @@ import os -import shutil import unittest from pathlib import Path -from unittest.mock import patch, MagicMock from src.validation import Validation +from unittest.mock import patch, MagicMock DOWNLOAD_FILE_PATH = f'{Path.cwd()}/downloads' SAVED_FILE_PATH = f'{Path.cwd()}/tests/unit_tests/test_files' @@ -21,466 +20,140 @@ WRONG_DATATYPE_FILE_NAME = 'wrong_datatype.zip' -class TestOtherValidation(unittest.TestCase): - - @patch.object(Validation, 'download_single_file') - def setUp(self, mock_download_single_file): - os.makedirs(DOWNLOAD_FILE_PATH, exist_ok=True) - source = f'{SAVED_FILE_PATH}/{SUCCESS_FILE_NAME}' - destination = f'{DOWNLOAD_FILE_PATH}/{SUCCESS_FILE_NAME}' - - if not os.path.isfile(destination): - shutil.copyfile(source, destination) - - file_path = f'{DOWNLOAD_FILE_PATH}/{SUCCESS_FILE_NAME}' - - with patch.object(Validation, '__init__', return_value=None): - self.validator = Validation(file_path=file_path, storage_client=MagicMock()) - self.validator.file_path = file_path - self.validator.file_relative_path = SUCCESS_FILE_NAME - self.validator.container_name = None - mock_download_single_file.return_value = file_path - - def tearDown(self): - pass - - def test_download_single_file(self): - # Arrange - file_upload_path = DOWNLOAD_FILE_PATH - self.validator.storage_client = MagicMock() - self.validator.storage_client.get_file_from_url = MagicMock() - file = MagicMock() - file.file_path = 'text_file.txt' - file.get_stream = MagicMock(return_value=b'file_content') - self.validator.storage_client.get_file_from_url.return_value = file - - # Act - downloaded_file_path = self.validator.download_single_file(file_upload_path=file_upload_path) - - # Assert - self.validator.storage_client.get_file_from_url.assert_called_once_with(self.validator.container_name, - file_upload_path) - file.get_stream.assert_called_once() - with open(downloaded_file_path, 'rb') as f: - content = f.read() - self.assertEqual(content, b'file_content') - - def test_clean_up_file(self): - # Arrange - file_upload_path = DOWNLOAD_FILE_PATH - text_file_path = f'{file_upload_path}/text_file.txt' - f = open(text_file_path, "w") - f.write("Sample text") - f.close() - - # Act - Validation.clean_up = MagicMock() - - # Assert - # self.assertFalse(os.path.exists(text_file_path)) - - def test_clean_up_folder(self): - # Arrange - directory_name = 'temp' - directory_path = f'{DOWNLOAD_FILE_PATH}/{directory_name}' - is_exists = os.path.exists(directory_path) - if not is_exists: - os.makedirs(directory_path) - - # Act - Validation.clean_up = MagicMock() - - # Assert - # self.assertFalse(os.path.exists(directory_name)) - - class TestValidation(unittest.TestCase): - @patch.object(Validation, 'download_single_file') - def setUp(self, mock_download_single_file): - os.makedirs(DOWNLOAD_FILE_PATH, exist_ok=True) - source = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - destination = f'{DOWNLOAD_FILE_PATH}/{FAILURE_FILE_NAME}' - - if not os.path.isfile(destination): - shutil.copyfile(source, destination) - - file_path = f'{DOWNLOAD_FILE_PATH}/{FAILURE_FILE_NAME}' - - with patch.object(Validation, '__init__', return_value=None): - self.validator = Validation(file_path=file_path, storage_client=MagicMock()) - self.validator.file_path = file_path - self.validator.file_relative_path = FAILURE_FILE_NAME - self.validator.container_name = None - mock_download_single_file.return_value = file_path - - def tearDown(self): - pass - - def test_validate_with_invalid_file(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - - def test_validate_with_invalid_file_with_default_error_counts(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - self.assertLessEqual(len(result.validation_message), 20) - - def test_validate_with_invalid_file_with_specific_error_counts(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate(max_errors=10) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - self.assertLessEqual(len(result.validation_message), 10) - - def test_is_osw_valid_with_invalid_zip_file(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_invalid_format_file(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_validate_with_id_missing_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{ID_MISSING_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_id_missing_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{ID_MISSING_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.is_osw_valid(max_errors=2) + @patch('src.validation.Settings') + def setUp(self, mock_settings): + # Mock Settings and storage client to avoid actual dependencies + mock_settings.return_value.event_bus.container_name = 'test_container' - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + self.mock_storage_client = MagicMock() - def test_validate_with_invalid_edges_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{EDGES_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Test-specific setup + self.file_path = '/path/to/test.zip' + self.validation = Validation(file_path=self.file_path, storage_client=self.mock_storage_client) - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_invalid_edges_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{EDGES_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_validate_with_invalid_nodes_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{NODES_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_invalid_nodes_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{NODES_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_validate_with_invalid_points_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{POINTS_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.validate() - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_invalid_points_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{POINTS_INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + @patch('os.makedirs') + @patch('os.path.exists', return_value=False) + @patch('src.validation.uuid.uuid1') + def test_validation_init_creates_unique_dir(self, mock_uuid, mock_exists, mock_makedirs): + """Test that the Validation class correctly creates directories.""" + mock_uuid.return_value.hex = 'fixeduuidhex' + unique_id = 'fixeduuidhex'[0:24] + expected_dir = os.path.join(Path.cwd(), 'downloads', unique_id) - # Act - result = self.validator.is_osw_valid(max_errors=2) + # Act by reinitializing to trigger the directory creation + self.validation = Validation(file_path=self.file_path, storage_client=self.mock_storage_client) - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + # Assert that os.makedirs was called for the unique dir path + mock_makedirs.assert_called_with(expected_dir) - def test_validate_with_invalid_files_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + @patch('src.validation.Validation.clean_up') + @patch('src.validation.Validation.download_single_file') + def test_validate_valid_zip(self, mock_download_file, mock_clean_up): + """Test the validate method for a valid zip file.""" + mock_download_file.return_value = f'{SAVED_FILE_PATH}/{SUCCESS_FILE_NAME}' # Act - result = self.validator.validate() + result = self.validation.validate(max_errors=10) - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + # Assert that validation is marked as valid + self.assertTrue(result.is_valid) + self.assertEqual(result.validation_message, '') - def test_is_osw_valid_with_invalid_files_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{INVALID_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Ensure clean_up is called twice (once for the file, once for the folder) + self.assertEqual(mock_clean_up.call_count, 2) - # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_validate_with_invalid_geometry_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{INVALID_GEOMETRY_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + @patch('src.validation.Validation.clean_up') + @patch('src.validation.Validation.download_single_file') + def test_validate_invalid_file(self, mock_download_file, mock_clean_up): + """Test the validate method for a invalid file.""" + mock_download_file.return_value = f'{SAVED_FILE_PATH}/{FAILURE_FILE_NAME}' # Act - result = self.validator.validate() + result = self.validation.validate(max_errors=10) - # Assert + # Assert that validation is marked as valid self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_is_osw_valid_with_invalid_geometry_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{INVALID_GEOMETRY_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() - - # Act - result = self.validator.is_osw_valid(max_errors=2) + self.assertIn('Validation error', result.validation_message) - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + # Ensure clean_up is called twice (once for the file, once for the folder) + self.assertEqual(mock_clean_up.call_count, 2) - def test_validate_with_missing_identifier_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{MISSING_IDENTIFIER_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + @patch('src.validation.OSWValidation') + @patch('src.validation.Validation.clean_up') + def test_validate_invalid_zip(self, mock_clean_up, mock_osw_validation): + """Test validate method for invalid zip file with errors.""" + # Mock the OSWValidation validate method to return errors + mock_validation_result = MagicMock() + mock_validation_result.is_valid = False + mock_validation_result.errors = 'Failed to validate because unknown file format' + mock_osw_validation.return_value.validate.return_value = mock_validation_result # Act - result = self.validator.validate() + result = self.validation.validate(max_errors=10) - # Assert + # Assert that validation failed and the error message is returned self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + self.assertEqual(result.validation_message, 'Failed to validate because unknown file format') - def test_is_osw_valid_with_missing_identifier_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{MISSING_IDENTIFIER_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Ensure clean_up is called twice (once for the file, once for the folder) + self.assertEqual(mock_clean_up.call_count, 1) - # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + @patch('src.validation.Validation.download_single_file', return_value=f'{SAVED_FILE_PATH}/{SUCCESS_FILE_NAME}') + @patch('gc.collect') + @patch('shutil.rmtree') + @patch('os.remove') + def test_gc_collect_called(self, mock_remove, mock_rmtree, mock_gc, mock_download_file): + """Test if garbage collection is called.""" + # Mock the storage client to return a bytes-like stream + file_mock = MagicMock() + file_mock.get_stream.return_value = b'file content' - def test_validate_with_no_entity_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{NO_ENTITY_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Replace self.validation.storage_client.get_file_from_url to return the mocked file object + self.validation.storage_client.get_file_from_url.return_value = file_mock - # Act - result = self.validator.validate() + # Simulate validation process + self.validation.validate(max_errors=10) - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + # Ensure garbage collection is called + mock_gc.assert_called() - def test_is_osw_valid_with_no_entity_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{NO_ENTITY_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Ensure cleanup methods are called for valid paths + mock_remove.assert_called_once_with(f'{SAVED_FILE_PATH}/{SUCCESS_FILE_NAME}') + @patch('os.remove') + @patch('os.path.isfile', return_value=True) + def test_clean_up_file(self, mock_isfile, mock_remove): + """Test the static clean_up method with a file path.""" # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + Validation.clean_up('/path/to/file.zip') - def test_validate_with_wrong_datatype_zip(self): - # Arrange - file_path = f'{SAVED_FILE_PATH}/{WRONG_DATATYPE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + # Assert that os.remove is called for a file + mock_remove.assert_called_once_with('/path/to/file.zip') + @patch('os.remove') + @patch('shutil.rmtree') + def test_clean_up_directory(self, mock_rmtree, mock_remove): + """Test the static clean_up method with a directory path.""" # Act - result = self.validator.validate() + Validation.clean_up('/path/to/directory') - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) + # Assert that shutil.rmtree is called for a directory + mock_rmtree.assert_called_once_with('/path/to/directory', ignore_errors=False) + mock_remove.assert_not_called() - def test_is_osw_valid_with_wring_datatype_zip(self): + @patch('src.validation.uuid.uuid1') + def test_get_unique_id(self, mock_uuid): + """Test get_unique_id to ensure correct ID generation.""" # Arrange - file_path = f'{SAVED_FILE_PATH}/{WRONG_DATATYPE_FILE_NAME}' - expected_downloaded_file_path = file_path - self.validator.download_single_file = MagicMock(return_value=expected_downloaded_file_path) - Validation.clean_up = MagicMock() + mock_uuid.return_value.hex = 'mockuuidhex' # Act - result = self.validator.is_osw_valid(max_errors=2) - - # Assert - self.assertFalse(result.is_valid) - self.assertIsInstance(result.validation_message, list) - Validation.clean_up.assert_called_once_with(file_path) - - def test_download_single_file_exception(self): - # Arrange - file_upload_path = DOWNLOAD_FILE_PATH - self.validator.storage_client = MagicMock() - self.validator.storage_client.get_file_from_url = MagicMock() - file = MagicMock() - file.file_path = 'text_file.txt' - file.get_stream = MagicMock(return_value=b'file_content') - self.validator.storage_client.get_file_from_url.return_value = file + unique_id = self.validation.get_unique_id() - # Act - downloaded_file_path = self.validator.download_single_file(file_upload_path=file_upload_path) - - # Assert - self.validator.storage_client.get_file_from_url.assert_called_once_with(self.validator.container_name, - file_upload_path) - file.get_stream.assert_called_once() - with open(downloaded_file_path, 'rb') as f: - content = f.read() - self.assertEqual(content, b'file_content') + # Assert that the unique ID is generated as expected + self.assertEqual(unique_id, 'mockuuidhex'[0:24]) if __name__ == '__main__': From 327c76cf7cc299b78d6bf8deea3362d27a0d29cd Mon Sep 17 00:00:00 2001 From: sujata-m Date: Mon, 14 Oct 2024 14:41:10 +0530 Subject: [PATCH 4/6] Fixed unit test cases --- tests/unit_tests/test_validation.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_validation.py b/tests/unit_tests/test_validation.py index b256d78..bcaa483 100644 --- a/tests/unit_tests/test_validation.py +++ b/tests/unit_tests/test_validation.py @@ -75,7 +75,8 @@ def test_validate_invalid_file(self, mock_download_file, mock_clean_up): # Assert that validation is marked as valid self.assertFalse(result.is_valid) - self.assertIn('Validation error', result.validation_message) + self.assertIn('Validation error', ' '.join(result.validation_message)) + # Ensure clean_up is called twice (once for the file, once for the folder) self.assertEqual(mock_clean_up.call_count, 2) @@ -100,6 +101,23 @@ def test_validate_invalid_zip(self, mock_clean_up, mock_osw_validation): # Ensure clean_up is called twice (once for the file, once for the folder) self.assertEqual(mock_clean_up.call_count, 1) + @patch('src.validation.Validation.download_single_file') + def test_validate_unknown_file_format(self, mock_download_file): + """Test validation failure for unknown file format.""" + + # Mock the file path with a non-zip extension to trigger the else block + self.validation.file_relative_path = 'invalid_file.txt' + + # Mock download to not affect the actual download method + mock_download_file.return_value = '/path/to/invalid_file.txt' + + # Act + result = self.validation.validate(max_errors=10) + + # Assert that the validation failed due to unknown file format + self.assertFalse(result.is_valid) + self.assertEqual(result.validation_message, 'Failed to validate because unknown file format') + @patch('src.validation.Validation.download_single_file', return_value=f'{SAVED_FILE_PATH}/{SUCCESS_FILE_NAME}') @patch('gc.collect') @patch('shutil.rmtree') From 8f1111b3bd396ec4002162451d955373fd707095 Mon Sep 17 00:00:00 2001 From: sujata-m Date: Tue, 29 Oct 2024 17:20:45 +0530 Subject: [PATCH 5/6] Fixed Issue 1377 - Added the specific version of `geopands` to avoid the breaking zone files --- requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/requirements.txt b/requirements.txt index 4d6c8c6..9af59c7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,4 +3,5 @@ pydantic==1.10.4 python-ms-core==0.0.22 uvicorn==0.20.0 html_testRunner==1.2.1 +geopands==0.14.4 python-osw-validation==0.2.7 \ No newline at end of file From afd2a75d68d195d35def1d53150b438868750669 Mon Sep 17 00:00:00 2001 From: sujata-m Date: Tue, 29 Oct 2024 17:28:02 +0530 Subject: [PATCH 6/6] fix typo --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 9af59c7..6a41b03 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,5 +3,5 @@ pydantic==1.10.4 python-ms-core==0.0.22 uvicorn==0.20.0 html_testRunner==1.2.1 -geopands==0.14.4 +geopandas==0.14.4 python-osw-validation==0.2.7 \ No newline at end of file