Merge pull request #190 from pittcsc/lab-changes

Minor refactoring of `lab.py` and `lab_test.py`
pittcsc · Aug 10, 2024 · 30d1fa0 · 30d1fa0
2 parents 6cdd9ea + d0ecbf6
commit 30d1fa0
Show file tree

Hide file tree

Showing 2 changed files with 96 additions and 82 deletions.
diff --git a/pittapi/lab.py b/pittapi/lab.py
@@ -19,7 +19,6 @@
 
 from typing import NamedTuple
 import requests
-from urllib.error import HTTPError
 
 # Suppress ssl warning
 import urllib3
@@ -71,17 +70,10 @@ def get_one_lab_data(lab_name: str) -> Lab:
         Lab: A Lab object with the data.
     """
 
-    valid_lab_names = [
-        "BELLEFIELD",
-        "LAWRENCE",
-        "SUTH",
-        "CATH_G27",
-        "CATH_G62",
-        "BENEDUM",
-    ]
-
-    if lab_name not in valid_lab_names:
-        raise ValueError(f"Invalid lab name: {lab_name}. Valid options: {', '.join(valid_lab_names)}")
+    if lab_name not in AVAIL_LAB_ID_MAP.keys():
+        # Dicts are guaranteed to preserve insertion order as of Python 3.7,
+        # so the list of valid options will always be printed in the same order
+        raise ValueError(f"Invalid lab name: {lab_name}. Valid options: {', '.join(AVAIL_LAB_ID_MAP.keys())}")
 
     req = requests.get(
         PITT_BASE_URL + AVAIL_LAB_ID_MAP[lab_name] + "/status.json?noredir=1",
@@ -109,6 +101,7 @@ def get_one_lab_data(lab_name: str) -> Lab:
     # Available: 1
     # In Use: 2
     # Out of Service Unknown (just going to use default condition to handle this)
+    # TODO: find out status number for Out of Service
     for computer_info in lab_data["state"].values():
         if computer_info["up"] == 0:
             off_computers += 1
@@ -137,9 +130,6 @@ def get_all_labs_data() -> list[Lab]:
         list[Lab]: A list of Labs.
     """
 
-    all_lab_data = []
-
-    for lab_name in AVAIL_LAB_ID_MAP.keys():
-        all_lab_data.append(get_one_lab_data(lab_name))
+    all_lab_data = [get_one_lab_data(lab_name) for lab_name in AVAIL_LAB_ID_MAP.keys()]
 
     return all_lab_data
diff --git a/tests/lab_test.py b/tests/lab_test.py
@@ -38,17 +38,21 @@ def test_get_status_bellefield(self):
             json=lab_mocks.mocked_bellefield_data,
         )
 
-        results = lab.get_one_lab_data("BELLEFIELD")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "Bellefield 314")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 29)
-        self.assertEqual(results.off_computers, 1)
-        self.assertEqual(results.in_use_computers, 0)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 30)
+        result = lab.get_one_lab_data("BELLEFIELD")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="Bellefield 314",
+                status=False,
+                available_computers=29,
+                off_computers=1,
+                in_use_computers=0,
+                out_of_service_computers=0,
+                total_computers=30,
+            ),
+        )
 
     @responses.activate
     def test_get_status_lawrence(self):
@@ -58,17 +62,21 @@ def test_get_status_lawrence(self):
             json=lab_mocks.mocked_lawrence_data,
         )
 
-        results = lab.get_one_lab_data("LAWRENCE")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "David Lawrence 230")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 25)
-        self.assertEqual(results.off_computers, 10)
-        self.assertEqual(results.in_use_computers, 5)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 40)
+        result = lab.get_one_lab_data("LAWRENCE")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="David Lawrence 230",
+                status=False,
+                available_computers=25,
+                off_computers=10,
+                in_use_computers=5,
+                out_of_service_computers=0,
+                total_computers=40,
+            ),
+        )
 
     @responses.activate
     def test_get_status_sutherland(self):
@@ -78,17 +86,21 @@ def test_get_status_sutherland(self):
             json=lab_mocks.mocked_sutherland_data,
         )
 
-        results = lab.get_one_lab_data("SUTH")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "Sutherland 120")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 11)
-        self.assertEqual(results.off_computers, 1)
-        self.assertEqual(results.in_use_computers, 0)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 12)
+        result = lab.get_one_lab_data("SUTH")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="Sutherland 120",
+                status=False,
+                available_computers=11,
+                off_computers=1,
+                in_use_computers=0,
+                out_of_service_computers=0,
+                total_computers=12,
+            ),
+        )
 
     @responses.activate
     def test_get_status_cathg27(self):
@@ -98,17 +110,21 @@ def test_get_status_cathg27(self):
             json=lab_mocks.mocked_cathy_g27_data,
         )
 
-        results = lab.get_one_lab_data("CATH_G27")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "Cathedral G27")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 16)
-        self.assertEqual(results.off_computers, 3)
-        self.assertEqual(results.in_use_computers, 11)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 30)
+        result = lab.get_one_lab_data("CATH_G27")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="Cathedral G27",
+                status=False,
+                available_computers=16,
+                off_computers=3,
+                in_use_computers=11,
+                out_of_service_computers=0,
+                total_computers=30,
+            ),
+        )
 
     @responses.activate
     def test_get_status_cathg62(self):
@@ -118,17 +134,21 @@ def test_get_status_cathg62(self):
             json=lab_mocks.mocked_cathy_g62_data,
         )
 
-        results = lab.get_one_lab_data("CATH_G62")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "Cathedral G62")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 26)
-        self.assertEqual(results.off_computers, 5)
-        self.assertEqual(results.in_use_computers, 0)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 31)
+        result = lab.get_one_lab_data("CATH_G62")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="Cathedral G62",
+                status=False,
+                available_computers=26,
+                off_computers=5,
+                in_use_computers=0,
+                out_of_service_computers=0,
+                total_computers=31,
+            ),
+        )
 
     @responses.activate
     def test_get_status_benedum(self):
@@ -138,17 +158,21 @@ def test_get_status_benedum(self):
             json=lab_mocks.mocked_benedum_data,
         )
 
-        results = lab.get_one_lab_data("BENEDUM")
-
-        self.assertIsInstance(results, lab.Lab)
-
-        self.assertEqual(results.name, "Benedum B06")
-        self.assertEqual(results.status, False)
-        self.assertEqual(results.available_computers, 28)
-        self.assertEqual(results.off_computers, 7)
-        self.assertEqual(results.in_use_computers, 4)
-        self.assertEqual(results.out_of_service_computers, 0)
-        self.assertEqual(results.total_computers, 39)
+        result = lab.get_one_lab_data("BENEDUM")
+
+        self.assertIsInstance(result, lab.Lab)
+        self.assertEqual(
+            result,
+            lab.Lab(
+                name="Benedum B06",
+                status=False,
+                available_computers=28,
+                off_computers=7,
+                in_use_computers=4,
+                out_of_service_computers=0,
+                total_computers=39,
+            ),
+        )
 
     @responses.activate
     def test_get_all_lab_data(self):