From 4b109a70b01ac48f1f118721337a97d61e5d53aa Mon Sep 17 00:00:00 2001 From: Daniel Markstedt Date: Tue, 25 Oct 2022 08:51:04 -0700 Subject: [PATCH] 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 | 6 ++--- python/web/src/templates/index.html | 8 +++--- python/web/src/web.py | 3 ++- python/web/src/web_utils.py | 38 +++++++-------------------- python/web/tests/api/test_misc.py | 13 +++++++++ python/web/tests/api/test_settings.py | 2 +- 6 files changed, 33 insertions(+), 37 deletions(-) diff --git a/python/web/src/templates/drives.html b/python/web/src/templates/drives.html index 059c26fd..28f2ab7b 100644 --- a/python/web/src/templates/drives.html +++ b/python/web/src/templates/drives.html @@ -13,7 +13,7 @@ {{ _("Description") }} {{ _("Action") }} -{% for hd in env['drive_properties']['hd_conf']|sort(attribute='name') %} +{% for hd in drive_properties['hd_conf']|sort(attribute='name') %} {% if hd.url != "" %} @@ -48,7 +48,7 @@ {{ _("Description") }} {{ _("Action") }} -{% for cd in env['drive_properties']['cd_conf']|sort(attribute='name') %} +{% for cd in drive_properties['cd_conf']|sort(attribute='name') %} {% if cd.url != "" %} @@ -88,7 +88,7 @@ {{ _("Description") }} {{ _("Action") }} -{% for rm in env['drive_properties']['rm_conf']|sort(attribute='name') %} +{% for rm in drive_properties['rm_conf']|sort(attribute='name') %} {% if rm.url != "" %} diff --git a/python/web/src/templates/index.html b/python/web/src/templates/index.html index 541fba70..96246f43 100644 --- a/python/web/src/templates/index.html +++ b/python/web/src/templates/index.html @@ -392,21 +392,21 @@ {{ _("None") }} {% if type == "SCCD" %} - {% for drive in env["drive_properties"]["cd_conf"] | sort(attribute='name') %} + {% for drive in drive_properties["cd_conf"] | sort(attribute='name') %} {% 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') %} {% 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') %} @@ -581,7 +581,7 @@ - {% for drive in env["drive_properties"]["hd_conf"] | sort(attribute='name') %} + {% for drive in drive_properties["hd_conf"] | sort(attribute='name') %} diff --git a/python/web/src/web.py b/python/web/src/web.py index 989a8ca7..6e79f405 100644 --- a/python/web/src/web.py +++ b/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"]), ) diff --git a/python/web/src/web_utils.py b/python/web/src/web_utils.py index 4c6d5ba5..02ba45d9 100644 --- a/python/web/src/web_utils.py +++ b/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" + # 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"]) + + if device.get("size"): + device["size_mb"] = f'{device["size"] / 1024 / 1024:,.2f}' + if device["device_type"] == "SCHD": - device["secure_name"] = secure_filename(device["name"]) - device["size_mb"] = FORMAT_FILTER.format(device["size"] / 1024 / 1024) 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 diff --git a/python/web/tests/api/test_misc.py b/python/web/tests/api/test_misc.py index ba8f1836..41db0280 100644 --- a/python/web/tests/api/test_misc.py +++ b/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"] diff --git a/python/web/tests/api/test_settings.py b/python/web/tests/api/test_settings.py index e50612d0..18d3d3b9 100644 --- a/python/web/tests/api/test_settings.py +++ b/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",