Improve Web UI integration tests (#939)

* Remove deprecated critital log level from test

* Move drive_properties back into template data sets

* Move properties data integrity checks to test code

* Streamline the drive formatting logic

python/web/src/templates/drives.html

@@ -13,7 +13,7 @@
     <th scope="col">{{ _("Description") }}</th>
     <th scope="col">{{ _("Action") }}</th>
 </tr>
-{% for hd in env['drive_properties']['hd_conf']|sort(attribute='name') %}
+{% for hd in drive_properties['hd_conf']|sort(attribute='name') %}
 <tr>
     <td align="center">
         {% if hd.url != "" %}
@@ -48,7 +48,7 @@
     <th scope="col">{{ _("Description") }}</th>
     <th scope="col">{{ _("Action") }}</th>
 </tr>
-{% for cd in env['drive_properties']['cd_conf']|sort(attribute='name') %}
+{% for cd in drive_properties['cd_conf']|sort(attribute='name') %}
 <tr>
     <td align="center">
         {% if cd.url != "" %}
@@ -88,7 +88,7 @@
     <th scope="col">{{ _("Description") }}</th>
     <th scope="col">{{ _("Action") }}</th>
 </tr>
-{% for rm in env['drive_properties']['rm_conf']|sort(attribute='name') %}
+{% for rm in drive_properties['rm_conf']|sort(attribute='name') %}
 <tr>
     <td align="center">
         {% if rm.url != "" %}

python/web/src/templates/index.html

@@ -392,21 +392,21 @@
                 {{ _("None") }}
                 </option>
                 {% if type == "SCCD" %}
-                {% for drive in env["drive_properties"]["cd_conf"] | sort(attribute='name') %}
+                {% for drive in drive_properties["cd_conf"] | sort(attribute='name') %}
                 <option value="{{ drive.name }}">
                 {{ drive.name }}
                 </option>
                 {% endfor %}
                 {% endif %}
                 {% if type == "SCRM" %}
-                {% for drive in env["drive_properties"]["rm_conf"] | sort(attribute='name') %}
+                {% for drive in drive_properties["rm_conf"] | sort(attribute='name') %}
                 <option value="{{ drive.name }}">
                 {{ drive.name }}
                 </option>
                 {% endfor %}
                 {% endif %}
                 {% if type == "SCMO" %}
-                {% for drive in env["drive_properties"]["mo_conf"] | sort(attribute='name') %}
+                {% for drive in drive_properties["mo_conf"] | sort(attribute='name') %}
                 <option value="{{ drive.name }}">
                 {{ drive.name }}
                 </option>
@@ -581,7 +581,7 @@
     <option value="">
     {{ _("None") }}
     </option>
-    {% for drive in env["drive_properties"]["hd_conf"] | sort(attribute='name') %}
+    {% for drive in drive_properties["hd_conf"] | sort(attribute='name') %}
     <option value="{{ drive.name }}">
     {{ drive.name }}
     </option>

python/web/src/web.py

@@ -98,7 +98,6 @@ def get_env_info():
         "cd_suffixes": tuple(server_info["sccd"]),
         "rm_suffixes": tuple(server_info["scrm"]),
         "mo_suffixes": tuple(server_info["scmo"]),
-        "drive_properties": format_drive_properties(APP.config["RASCSI_DRIVE_PROPERTIES"]),
     }
 
 
@@ -249,6 +248,7 @@ def index():
         image_suffixes_to_create=image_suffixes_to_create,
         valid_image_suffixes=valid_image_suffixes,
         max_file_size=int(int(MAX_FILE_SIZE) / 1024 / 1024),
+        drive_properties=format_drive_properties(APP.config["RASCSI_DRIVE_PROPERTIES"]),
         RESERVATIONS=RESERVATIONS,
         CFG_DIR=CFG_DIR,
         AFP_DIR=AFP_DIR,
@@ -278,6 +278,7 @@ def drive_list():
     return response(
         template="drives.html",
         files=file_cmd.list_images()["files"],
+        drive_properties=format_drive_properties(APP.config["RASCSI_DRIVE_PROPERTIES"]),
         )
 
 

python/web/src/web_utils.py

@@ -153,29 +153,25 @@ def format_drive_properties(drive_properties):
     cd_conf = []
     rm_conf = []
     mo_conf = []
-    FORMAT_FILTER = "{:,.2f}"
 
     for device in drive_properties:
-        # Add fallback device names, since other code relies on this data for display
-        if not device["name"]:
-            if device["product"]:
-                device["name"] = device["product"]
-            else:
-                device["name"] = "Unknown Device"
-        if device["device_type"] == "SCHD":
+        # Fallback for when the properties data is corrupted, to avoid crashing the web app.
+        # The integration tests will catch this scenario, but relies on the web app not crashing.
+        if not device.get("name"):
+            device["name"] = ""
+
         device["secure_name"] = secure_filename(device["name"])
-            device["size_mb"] = FORMAT_FILTER.format(device["size"] / 1024 / 1024)
+
+        if device.get("size"):
+            device["size_mb"] = f'{device["size"] / 1024 / 1024:,.2f}'
+
+        if device["device_type"] == "SCHD":
             hd_conf.append(device)
         elif device["device_type"] == "SCCD":
-            device["size_mb"] = _("N/A")
             cd_conf.append(device)
         elif device["device_type"] == "SCRM":
-            device["secure_name"] = secure_filename(device["name"])
-            device["size_mb"] = FORMAT_FILTER.format(device["size"] / 1024 / 1024)
             rm_conf.append(device)
         elif device["device_type"] == "SCMO":
-            device["secure_name"] = secure_filename(device["name"])
-            device["size_mb"] = FORMAT_FILTER.format(device["size"] / 1024 / 1024)
             mo_conf.append(device)
 
     return {
@@ -193,21 +189,7 @@ def get_properties_by_drive_name(drives, drive_name):
     drives.sort(key=lambda item: item.get("name"))
 
     drive_props = None
-    prev_drive = {"name": ""}
     for drive in drives:
-        # TODO: Make this check into an integration test
-        if "name" not in drive:
-            logging.warning(
-                "Device without a name exists in the drive properties database. This is a bug."
-                )
-            break
-        # TODO: Make this check into an integration test
-        if drive["name"] == prev_drive["name"]:
-            logging.warning(
-                "Device with duplicate name \"%s\" in drive properties database. This is a bug.",
-                drive["name"],
-                )
-        prev_drive = drive
         if drive["name"] == drive_name:
             drive_props = drive
 

python/web/tests/api/test_misc.py

@@ -39,6 +39,19 @@ def test_show_named_drive_presets(http_client):
     response = http_client.get("/drive/list")
     response_data = response.json()
 
+    prev_drive = {"name": ""}
+    for drive in (
+        response_data["data"]["drive_properties"]["hd_conf"]
+        + response_data["data"]["drive_properties"]["cd_conf"]
+        + response_data["data"]["drive_properties"]["rm_conf"]
+        + response_data["data"]["drive_properties"]["mo_conf"]
+    ):
+        # Test that the named drive has a name
+        assert drive["name"] != ""
+        # Test that "name" is unique for each named drive
+        assert drive["name"] != prev_drive["name"]
+        prev_drive = drive
+
     assert response.status_code == 200
     assert response_data["status"] == STATUS_SUCCESS
     assert "files" in response_data["data"]

python/web/tests/api/test_settings.py

@@ -31,7 +31,7 @@ def test_set_language(http_client, locale, confirm_message):
 
 
 # route("/logs/level", methods=["POST"])
-@pytest.mark.parametrize("level", ["trace", "debug", "info", "warn", "err", "critical", "off"])
+@pytest.mark.parametrize("level", ["trace", "debug", "info", "warn", "err", "off"])
 def test_set_log_level(http_client, level):
     response = http_client.post(
         "/logs/level",