Merge pull request #164 from CDCgov/unit_tests

Unit tests integrated with Github workflow
CDCgov · Nov 21, 2024 · 17317dd · 17317dd
2 parents 0be3553 + 77536c9
commit 17317dd
Show file tree

Hide file tree

Showing 9 changed files with 98 additions and 141 deletions.
diff --git a/.github/workflows/ci.yml → .github/workflows/ci.yaml b/.github/workflows/ci.yml → .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
diff --git 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
@@ -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
@@ -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"]
diff --git 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
@@ -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