Address PR review: use fixtures in tests and simplify _load_pi_config

Author: marcoagonzales007

process_report/loader.py

@@ -114,20 +114,18 @@ def load_dataframe(self, filepath: str) -> pandas.DataFrame:
         return pandas.read_csv(filepath)
 
     @functools.lru_cache
-    def _load_pi_config(self, filepath: str) -> PIList:
-        with open(filepath) as file:
+    def _load_pi_config(self) -> PIList:
+        with open(invoice_settings.nonbillable_pis_filepath) as file:
             pi_list = yaml.safe_load(file)
 
         return PIList.model_validate(pi_list)
 
     def get_nonbillable_pis(self) -> list[str]:
-        pi_list = self._load_pi_config(invoice_settings.nonbillable_pis_filepath)
-        return pi_list.get_nonbillable_pis()
+        return self._load_pi_config().get_nonbillable_pis()
 
     def get_pi_non_billed_su_types(self) -> dict[str, list[str]]:
         """PI usernames -> list of SU types that receive credit (zeroed out)."""
-        pi_list = self._load_pi_config(invoice_settings.nonbillable_pis_filepath)
-        return pi_list.get_pi_non_billed_su_types()
+        return self._load_pi_config().get_pi_non_billed_su_types()
 
     @functools.lru_cache
     def get_nonbillable_projects(self) -> pandas.DataFrame:

process_report/tests/unit/test_nonbillable_models.py

@@ -3,39 +3,42 @@
 from process_report.models import nonbillable_models
 
 
-@pytest.mark.parametrize(
-    "projects_data",
-    [
-        [
-            {
-                "name": "ProjectA",
-                "clusters": [{"name": "Cluster1"}, {"name": "Cluster2"}],
-            },
-            {
-                "name": "ProjectB",
-                "clusters": [
-                    {"name": "Cluster1", "start": "2023-01", "end": "2023-12"}
-                ],
-            },
-            {"name": "ProjectC", "start": "2023-06", "end": "2023-07"},
-            {
-                "name": "ProjectD",
-                "is_billable": True,
-                "clusters": [
-                    {"name": "Cluster1", "start": "2023-05", "end": "2023-09"},
-                    {"name": "Cluster2", "start": "2023-05", "end": "2023-11"},
-                ],
-            },
-            {"name": "ProjectE"},
-        ],
-    ],
-)
-def test_timed_projects(mocker, projects_data):
-    mocker.patch(
+@pytest.fixture
+def sample_project_list():
+    return [
+        {
+            "name": "ProjectA",
+            "clusters": [{"name": "Cluster1"}, {"name": "Cluster2"}],
+        },
+        {
+            "name": "ProjectB",
+            "clusters": [{"name": "Cluster1", "start": "2023-01", "end": "2023-12"}],
+        },
+        {"name": "ProjectC", "start": "2023-06", "end": "2023-07"},
+        {
+            "name": "ProjectD",
+            "is_billable": True,
+            "clusters": [
+                {"name": "Cluster1", "start": "2023-05", "end": "2023-09"},
+                {"name": "Cluster2", "start": "2023-05", "end": "2023-11"},
+            ],
+        },
+        {"name": "ProjectE"},
+    ]
+
+
+@pytest.fixture
+def mock_allowed_clusters(mocker):
+    return mocker.patch(
         "process_report.models.nonbillable_models.get_allowed_clusters",
         return_value={"Cluster1", "Cluster2"},
     )
-    projects = nonbillable_models.ExcludedProjectList.model_validate(projects_data)
+
+
+def test_timed_projects(mock_allowed_clusters, sample_project_list):
+    projects = nonbillable_models.ExcludedProjectList.model_validate(
+        sample_project_list
+    )
     result = projects.get_nonbillable_projects("2023-09")
     timed = [(name, cluster) for name, cluster, is_timed, _ in result if is_timed]
     expected = [
@@ -46,39 +49,10 @@ def test_timed_projects(mocker, projects_data):
     assert timed == expected
 
 
-@pytest.mark.parametrize(
-    "projects_data",
-    [
-        [
-            {
-                "name": "ProjectA",
-                "clusters": [{"name": "Cluster1"}, {"name": "Cluster2"}],
-            },
-            {
-                "name": "ProjectB",
-                "clusters": [
-                    {"name": "Cluster1", "start": "2023-01", "end": "2023-12"}
-                ],
-            },
-            {"name": "ProjectC", "start": "2023-06", "end": "2023-07"},
-            {
-                "name": "ProjectD",
-                "is_billable": True,
-                "clusters": [
-                    {"name": "Cluster1", "start": "2023-05", "end": "2023-09"},
-                    {"name": "Cluster2", "start": "2023-05", "end": "2023-11"},
-                ],
-            },
-            {"name": "ProjectE"},
-        ],
-    ],
-)
-def test_nonbillable_projects(mocker, projects_data):
-    mocker.patch(
-        "process_report.models.nonbillable_models.get_allowed_clusters",
-        return_value={"Cluster1", "Cluster2"},
+def test_nonbillable_projects(mock_allowed_clusters, sample_project_list):
+    projects = nonbillable_models.ExcludedProjectList.model_validate(
+        sample_project_list
     )
-    projects = nonbillable_models.ExcludedProjectList.model_validate(projects_data)
     result = projects.get_nonbillable_projects("2023-09")
     expected = [
         ("ProjectA", "Cluster1", False, False),