diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yaml similarity index 62% rename from .github/workflows/ci.yml rename to .github/workflows/ci.yaml index 1929f8e..a1aef90 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yaml @@ -1,17 +1,15 @@ name: Python Unit Tests with Coverage -on: +on: push: - branches: [ unit_tests ] - pull_request: - branches: [ unit_tests ] + branches: '**' jobs: build: runs-on: ubuntu-latest strategy: matrix: - python-version: ["3.10", "3.11", "3.12", "3.13"] + python-version: ["3.10", "3.11", "3.12"] steps: - name: Checkout code @@ -22,11 +20,11 @@ jobs: with: python-version: ${{ matrix.python-version }} - - name: install poetry - run: pip install poetry + - name: Install test libraries + run: pip install -r tests/requirements.test.txt - - name: install package - run: poetry install --with test + - name: Install poetry package packages + run: poetry install - name: Run tests with coverage run: coverage run -m pytest diff --git a/.github/workflows/pre-commit.yaml b/.github/workflows/pre-commit.yaml deleted file mode 100644 index ea064a2..0000000 --- a/.github/workflows/pre-commit.yaml +++ /dev/null @@ -1,14 +0,0 @@ -name: pre-commit - -on: - pull_request: - push: - branches: [main] - -jobs: - pre-commit: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-python@v3 - - uses: ./.github/actions/pre-commit diff --git a/cfa_azure/clients.py b/cfa_azure/clients.py index d98e4ec..8726708 100644 --- a/cfa_azure/clients.py +++ b/cfa_azure/clients.py @@ -38,6 +38,7 @@ def __init__(self, config_path: str = None, use_env_vars: bool = False): self.pool_parameters = None self.timeout = None self.save_logs_to_blob = None + self.logs_folder = "stdout_stderr" logger.debug("Attributes initialized in client.") @@ -478,10 +479,8 @@ def update_containers( sleep(5.0) else: - logger.info( - f"Pool {pool_name} does not exist. New pool will be created." - ) - + logger.info(f"Pool {pool_name} does not exist. New pool will be created.") + container_image_name=self.container_image_name if "pool_id" not in self.config["Batch"]: self.config["Batch"]["pool_id"] = pool_name @@ -595,9 +594,8 @@ def update_container_set( sleep(5.0) else: - logger.info( - f"Pool {pool_name} does not exist. New pool will be created." - ) + logger.info(f"Pool {pool_name} does not exist. New pool will be created.") + container_image_name=self.container_image_name if "pool_id" not in self.config["Batch"]: self.config["Batch"]["pool_id"] = pool_name diff --git a/cfa_azure/helpers.py b/cfa_azure/helpers.py index 617099f..a4a1881 100644 --- a/cfa_azure/helpers.py +++ b/cfa_azure/helpers.py @@ -1985,10 +1985,8 @@ def check_config_req(config: str): return True else: logger.warning( - f"{str(list(req - loaded))} keys missing from the config file and may be required by client." - ) - print( - f"{str(list(req - loaded))} keys missing from the config file and may be required by client." + "%s missing from the config file and will be required by client.", + str(list(req - loaded)) ) return False diff --git a/pyproject.toml b/pyproject.toml index 0a72f38..5db162b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,3 +25,7 @@ pyyaml = "*" [build-system] requires = ["poetry-core"] build-backend = "poetry.core.masonry.api" + +[tool.pytest.ini_options] +testpaths = ["tests"] +python_files = ["*_tests.py"] \ No newline at end of file diff --git a/tests/clients_tests.py b/tests/clients_tests.py index 3efeac0..523d8e8 100644 --- a/tests/clients_tests.py +++ b/tests/clients_tests.py @@ -9,9 +9,7 @@ class TestClients(unittest.TestCase): @patch("cfa_azure.clients.logger") - @patch( - "cfa_azure.helpers.read_config", MagicMock(return_value=FAKE_CONFIG) - ) + @patch("cfa_azure.helpers.read_config", MagicMock(return_value=FAKE_CONFIG_MINIMAL)) @patch("cfa_azure.helpers.check_config_req", MagicMock(return_value=True)) @patch("cfa_azure.helpers.get_sp_secret", MagicMock(return_value=True)) @patch("cfa_azure.helpers.get_sp_credential", MagicMock(return_value=True)) @@ -37,6 +35,7 @@ def setUp(self, mock_logger): ) @patch("cfa_azure.clients.logger") + @patch("cfa_azure.helpers.get_deployment_config", MagicMock(return_value={"virtualMachineConfiguration": {}})) def test_set_pool_info(self, mock_logger): self.azure_client.set_pool_info( mode="fixed", @@ -99,14 +98,8 @@ def test_add_task_inputfiles(self): ) self.assertIsNotNone(task_list) - @patch( - "cfa_azure.helpers.check_azure_container_exists", - MagicMock(return_value=FAKE_CONTAINER_IMAGE), - ) - @patch( - "cfa_azure.helpers.get_pool_full_info", - MagicMock(return_value=dict2obj(FAKE_POOL_INFO)), - ) + @patch("cfa_azure.helpers.check_azure_container_exists", MagicMock(return_value=FAKE_CONTAINER_IMAGE)) + @patch("cfa_azure.helpers.get_pool_full_info", MagicMock(return_value=FakeClient.FakePool.FakePoolInfo())) def test_add_task_dependencies(self): self.azure_client.pool_name = FAKE_BATCH_POOL task_1 = self.azure_client.add_task( @@ -156,6 +149,7 @@ def test_set_debugging_disable(self, mock_logger): self.assertFalse(self.azure_client.debug) mock_logger.debug.assert_called_with("Debugging turned off.") + @patch("cfa_azure.helpers.get_deployment_config", MagicMock(return_value={"virtualMachineConfiguration": {}})) def test_create_pool(self): self.azure_client.set_pool_info(mode="autoscale") pool_details = self.azure_client.create_pool(FAKE_BATCH_POOL) @@ -432,15 +426,9 @@ def test_check_job_status_noexist(self, mock_logger): @patch("cfa_azure.clients.logger") @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) def test_update_container_set(self, mock_logger): containers = [ {"name": FAKE_INPUT_CONTAINER, "relative_mount_dir": "input"}, @@ -456,19 +444,11 @@ def test_update_container_set(self, mock_logger): self.assertIsNone(pool_name) @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.format_rel_path", - MagicMock(return_value="/some_path"), - ) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.format_rel_path", MagicMock(return_value="/some_path")) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) + @patch("cfa_azure.helpers.get_sp_secret", MagicMock(return_value=True)) def test_update_container_set_forced(self): self.azure_client.blob_service_client = FakeClient() containers = [ @@ -481,18 +461,10 @@ def test_update_container_set_forced(self): ) self.assertEqual(pool_name, FAKE_BATCH_POOL) - @patch( - "cfa_azure.helpers.check_pool_exists", MagicMock(return_value=False) - ) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=False)) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) def test_update_containers_new_pool(self): containers = [ {"name": FAKE_INPUT_CONTAINER, "relative_mount_dir": "input"}, @@ -508,15 +480,9 @@ def test_update_containers_new_pool(self): @patch("cfa_azure.clients.logger") @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) def test_update_containers(self, mock_logger): pool_name = self.azure_client.update_containers( input_container_name=FAKE_INPUT_CONTAINER, @@ -528,23 +494,11 @@ def test_update_containers(self, mock_logger): ) self.assertIsNone(pool_name) - @patch("cfa_azure.clients.logger") - @patch( - "cfa_azure.clients.AzureClient.get_container_image_name", - MagicMock(return_value=FAKE_CONTAINER_IMAGE), - ) @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch( - "cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient()) - ) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) + @patch("cfa_azure.helpers.get_sp_secret", MagicMock(return_value=True)) def test_update_containers_forced(self): pool_name = self.azure_client.update_containers( pool_name=FAKE_BATCH_POOL, @@ -554,18 +508,10 @@ def test_update_containers_forced(self): ) self.assertEqual(pool_name, FAKE_BATCH_POOL) - @patch( - "cfa_azure.helpers.check_pool_exists", MagicMock(return_value=False) - ) - @patch( - "cfa_azure.helpers.get_batch_service_client", - MagicMock(return_value=FakeClient()), - ) - @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=True)) - @patch( - "cfa_azure.helpers.create_batch_pool", - MagicMock(return_value=FAKE_BATCH_POOL), - ) + @patch("cfa_azure.helpers.check_pool_exists", MagicMock(return_value=False)) + @patch("cfa_azure.helpers.get_batch_service_client", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.delete_pool", MagicMock(return_value=FakeClient())) + @patch("cfa_azure.helpers.create_batch_pool", MagicMock(return_value=FAKE_BATCH_POOL)) def test_update_containers_new_pool(self): pool_name = self.azure_client.update_containers( pool_name=FAKE_BATCH_POOL, diff --git a/tests/fake_client.py b/tests/fake_client.py index b819fe3..8bd4e15 100644 --- a/tests/fake_client.py +++ b/tests/fake_client.py @@ -102,7 +102,6 @@ "mount_configuration": {}, } - class FakeClient: class FakeBatchJob: def delete(self, *args): @@ -130,7 +129,7 @@ class FakeTask: def state(self): return batchmodels.TaskState.completed - def add(self, job_id, task, exit_conditions: dict): + def add(self, job_id, task): return True def as_dict(self): @@ -184,12 +183,33 @@ def get_secret(self, vault_sp_secret_id=None): class FakePool: class FakePoolInfo: + class FakeScaleSettings: + @property + def auto_scale(self): + return "fixed" + + def as_dict(self): + return FAKE_POOL_INFO + class FakeDeploymentConfig: class VMConfiguration: class ContainerConfig: + class FakeContainerRegistry: + @property + def registry_server(self): + return "registry_server" + + @property + def user_name(self): + return "user_name" + @property def container_image_names(self): return [FAKE_CONTAINER_IMAGE] + + @property + def container_registries(self): + return [self.FakeContainerRegistry()] @property def container_configuration(self): @@ -222,6 +242,10 @@ def last_modified(self): @property def vm_size(self): return FAKE_POOL_SIZE + + @property + def scale_settings(self): + return self.FakeScaleSettings() def get(self): return True diff --git a/tests/helpers_tests.py b/tests/helpers_tests.py index 85949a8..6bd7230 100644 --- a/tests/helpers_tests.py +++ b/tests/helpers_tests.py @@ -436,7 +436,7 @@ def test_get_completed_tasks(self): self.assertEqual(task_summary["completed tasks"], 1) def test_check_config_req(self): - status = cfa_azure.helpers.check_config_req(FAKE_CONFIG) + status = cfa_azure.helpers.check_config_req(FAKE_CONFIG_MINIMAL) self.assertIsNotNone(status) def test_check_config_req_badconfig(self): @@ -547,21 +547,6 @@ def test_download_directory_extensions(self, mock_logger): ) mock_logger.debug.assert_called_with("Download complete.") - @patch("cfa_azure.helpers.logger") - @patch( - "cfa_azure.helpers.check_blob_existence", MagicMock(return_value=True) - ) - def test_download_file(self, mock_logger): - blob_service_client = FakeClient() - cfa_azure.helpers.download_file( - c_client=blob_service_client, - src_path="some_path/", - dest_path="/another_path", - do_check=False, - verbose=False, - ) - mock_logger.debug.assert_called_with("File downloaded.") - @patch("cfa_azure.helpers.logger") @patch("cfa_azure.helpers.download_file", MagicMock(return_value=True)) def test_download_directory_extensions_inclusions(self, mock_logger): @@ -665,17 +650,15 @@ def test_check_azure_container_exists_missing_tag(self): ) self.assertIsNone(response) - @patch( - "cfa_azure.helpers.get_autoscale_formula", - MagicMock(return_value=FAKE_AUTOSCALE_FORMULA), - ) + @patch("cfa_azure.helpers.get_autoscale_formula", MagicMock(return_value=FAKE_AUTOSCALE_FORMULA)) + @patch("cfa_azure.helpers.get_deployment_config", MagicMock(return_value={"virtualMachineConfiguration": {}})) def test_get_pool_parameters(self): response = cfa_azure.helpers.get_pool_parameters( mode="autoscale", container_image_name=FAKE_CONTAINER_IMAGE, container_registry_url=FAKE_CONTAINER_REGISTRY, container_registry_server=FAKE_CONTAINER_REGISTRY, - config=FAKE_CONFIG, + config=FAKE_CONFIG_MINIMAL, mount_config=[], autoscale_formula_path="some_autoscale_formula", timeout=60, @@ -686,13 +669,14 @@ def test_get_pool_parameters(self): ) self.assertIsNotNone(response) + @patch("cfa_azure.helpers.get_deployment_config", MagicMock(return_value={"virtualMachineConfiguration": {}})) def test_get_pool_parameters_use_default(self): response = cfa_azure.helpers.get_pool_parameters( mode="autoscale", container_image_name=FAKE_CONTAINER_IMAGE, container_registry_url=FAKE_CONTAINER_REGISTRY, container_registry_server=FAKE_CONTAINER_REGISTRY, - config=FAKE_CONFIG, + config=FAKE_CONFIG_MINIMAL, mount_config=[], autoscale_formula_path="some_autoscale_formula", timeout=60, @@ -709,7 +693,7 @@ def test_get_pool_parameters_bad_mode(self): container_image_name=FAKE_CONTAINER_IMAGE, container_registry_url=FAKE_CONTAINER_REGISTRY, container_registry_server=FAKE_CONTAINER_REGISTRY, - config=FAKE_CONFIG, + config=FAKE_CONFIG_MINIMAL, mount_config=[], autoscale_formula_path="some_autoscale_formula", timeout=60, diff --git a/tests/requirements.test.txt b/tests/requirements.test.txt new file mode 100644 index 0000000..6249f62 --- /dev/null +++ b/tests/requirements.test.txt @@ -0,0 +1,19 @@ +azure-identity==1.16.1 +azure-keyvault==4.2.0 +azure-batch==14.0.0 +azure-mgmt-batch==17.1.0 +azure-storage-blob==12.17.0 +azure-containerregistry==1.2.0 +callee +coverage +cryptography>=43.0.1 +docker +numpy>=2.1.1 +setuptools +pandas +pathlib +poetry +pytest +pytest-xdist +pyyaml +toml==0.10.2 \ No newline at end of file