Chore: [ELI-751] - Error handling

Makefile

@@ -97,3 +97,6 @@ endif
 
 run-vita-preprod-tests:
 	poetry run pytest --env=preprod --log-cli-level=info tests/test_vita_integration_tests.py tests/test_upload_consumer_configs.py
+
+run-unit-tests: guard-env guard-log_level
+	poetry run pytest --env=${env} --log-cli-level=${log_level} tests/test_error_handling_utils.py -v

tests/performance_tests/locust.py

@@ -1,6 +1,7 @@
 import ast
 import csv
 import os
+import sys
 from pathlib import Path
 
 import urllib3
@@ -18,10 +19,22 @@ def _(parser):
     )
 
 
-with open("temp/nhs_numbers.csv", newline="") as csvFile:
-    reader = csv.reader(csvFile)
-    next(reader, None)
-    csvData = list(reader)
+csvData = []
+try:
+    with open("temp/nhs_numbers.csv", newline="") as csvFile:
+        reader = csv.reader(csvFile)
+        next(reader, None)  # Skip header
+        csvData = list(reader)
+
+    if not csvData:
+        print("Error: temp/nhs_numbers.csv is empty.", file=sys.stderr)
+except FileNotFoundError:
+    print(
+        "Error: temp/nhs_numbers.csv not found. Ensure test data is generated.",
+        file=sys.stderr,
+    )
+except (OSError, IOError) as e:
+    print(f"Error reading temp/nhs_numbers.csv: {e}", file=sys.stderr)
 
 
 # Class for API execution

tests/performance_tests/test_performance_tests.py

@@ -295,6 +295,7 @@ def test_locust_run_and_csv_exists(
     xray_sampling_rate,
     perf_mapping_upload,
 ):
+
     custom_env = os.environ.copy()
     custom_env["BASE_URL"] = eligibility_client.api_url
 
@@ -309,7 +310,14 @@ def test_locust_run_and_csv_exists(
     start_time = datetime.now(timezone.utc)
     logging.warning("LOCUST TEST STARTING: start_time=%s", start_time)
 
-    proc = _run_locust(locust_command, env=custom_env)
+    try:
+        proc = _run_locust(locust_command, env=custom_env)
+    except FileNotFoundError:
+        pytest.fail(
+            "Locust executable not found. Ensure locust is installed and in PATH."
+        )
+    except subprocess.SubprocessError as e:
+        pytest.fail(f"Failed to execute Locust subprocess: {e}")
 
     end_time = datetime.now(timezone.utc)
     logging.warning("LOCUST TEST FINISHED: end_time=%s", end_time)

tests/performance_tests/validate_inputs.py

@@ -35,6 +35,7 @@ def parse_run_time_to_seconds(run_time: str) -> int:
             "run_time must look like 30s or 5m (seconds or minutes only). "
             f"Got: '{run_time}'",
         )
+        return 0
 
     value = int(match.group(1))
     unit = match.group(2)

tests/test_error_handling_utils.py

@@ -0,0 +1,136 @@
+import os
+from unittest.mock import MagicMock, patch
+
+import pytest
+from botocore.exceptions import ClientError
+
+from utils.data_helper import load_all_expected_responses, load_all_test_scenarios
+from utils.dynamo_helper import file_backup_exists
+from utils.eligibility_api_client import EligibilityApiClient
+from tests.performance_tests.validate_inputs import parse_run_time_to_seconds
+
+# ---------------------------------------------------------------------------
+# 1. data_helper.py — load_all_expected_responses
+# ---------------------------------------------------------------------------
+
+
+def test_load_all_expected_responses_skips_bad_json(tmp_path):
+    """A corrupt JSON file is skipped; valid files in the same folder still load."""
+    (tmp_path / "bad.json").write_text("NOT JSON", encoding="utf-8")
+    (tmp_path / "good.json").write_text('{"id": "123", "data": "ok"}', encoding="utf-8")
+
+    result = load_all_expected_responses(tmp_path)
+
+    assert "good.json" in result
+    assert "bad.json" not in result
+
+
+# ---------------------------------------------------------------------------
+# 2. data_helper.py — load_all_test_scenarios
+# ---------------------------------------------------------------------------
+
+
+def test_load_all_test_scenarios_skips_missing_data_key(tmp_path):
+    """A scenario file that has no 'data' key is skipped; the result is an empty dict.
+
+    The TemplateEngine is mocked so that no real template files are needed.
+    Its apply() return value is configured to return the input unchanged so
+    that placeholder resolution does not fail if the 'data' key were present.
+    """
+    (tmp_path / "scenario.json").write_text('{"other_key": "value"}', encoding="utf-8")
+
+    mock_engine = MagicMock()
+    mock_engine.apply.side_effect = lambda data: data  # pass-through
+
+    with patch("utils.data_helper.TemplateEngine.create", return_value=mock_engine):
+        result = load_all_test_scenarios(tmp_path)
+
+    assert result == {}
+
+
+def test_load_all_test_scenarios_skips_bad_json(tmp_path):
+    """A scenario file with invalid JSON is skipped; the result is an empty dict."""
+    (tmp_path / "scenario.json").write_text("{ bad json", encoding="utf-8")
+
+    mock_engine = MagicMock()
+    mock_engine.apply.side_effect = lambda data: data
+
+    with patch("utils.data_helper.TemplateEngine.create", return_value=mock_engine):
+        result = load_all_test_scenarios(tmp_path)
+
+    assert result == {}
+
+
+# ---------------------------------------------------------------------------
+# 3. dynamo_helper.py — file_backup_exists
+# ---------------------------------------------------------------------------
+
+
+@patch("utils.dynamo_helper.load_from_file")
+def test_file_backup_exists_catches_json_decode_error(mock_load_from_file):
+    """file_backup_exists returns False (not an exception) when a backup file is corrupted."""
+    # All load_from_file calls return a string that is not valid JSON
+    mock_load_from_file.return_value = "NOT JSON"
+
+    mock_db = MagicMock()
+    mock_db.environment = "test"
+
+    result = file_backup_exists(mock_db)
+
+    assert result is False
+
+
+@patch.dict(os.environ, {"BASE_URL": "http://localhost"}, clear=True)
+@patch("utils.eligibility_api_client.boto3.client")
+def test_api_client_handles_ssm_client_error(mock_boto_client):
+    """A boto3 ClientError from SSM is wrapped in a descriptive RuntimeError."""
+    mock_ssm = MagicMock()
+    error_response = {
+        "Error": {"Code": "AccessDeniedException", "Message": "Access Denied"}
+    }
+    mock_ssm.get_parameter.side_effect = ClientError(error_response, "GetParameter")
+    mock_boto_client.return_value = mock_ssm
+
+    # Force _ensure_certs_present to think no certs exist on disk
+    with patch("pathlib.Path.exists", return_value=False):
+        with pytest.raises(RuntimeError, match="Error retrieving .* from SSM"):
+            EligibilityApiClient()
+
+
+# ---------------------------------------------------------------------------
+# 4. validate_inputs.py — parse_run_time_to_seconds and main()
+# ---------------------------------------------------------------------------
+
+
+def test_parse_run_time_to_seconds_valid():
+    """Seconds and minutes are parsed correctly."""
+    assert parse_run_time_to_seconds("30s") == 30
+    assert parse_run_time_to_seconds("5m") == 300
+
+
+@patch("tests.performance_tests.validate_inputs.fail")
+def test_parse_run_time_to_seconds_invalid(mock_fail):
+    """An unsupported unit calls fail() with the right message and returns 0 (not an exception)."""
+    result = parse_run_time_to_seconds("10h")
+
+    mock_fail.assert_called_once_with(
+        "Invalid run_time format",
+        "run_time must look like 30s or 5m (seconds or minutes only). Got: '10h'",
+    )
+    # After fail() is mocked (no sys.exit), the function returns the sentinel 0
+    assert result == 0
+
+
+@patch("tests.performance_tests.validate_inputs.get_env")
+@patch("tests.performance_tests.validate_inputs.fail")
+def test_validate_inputs_main_catches_value_error(mock_fail, mock_get_env):
+    """A non-integer USERS value calls fail() and returns early — no NameError or crash."""
+    from tests.performance_tests.validate_inputs import main
+
+    # USERS returns a non-integer; all other env vars return a valid integer string
+    mock_get_env.side_effect = lambda name: "NOT_AN_INT" if name == "USERS" else "1"
+
+    # Should complete without raising any exception
+    main()
+
+    mock_fail.assert_called_once_with("Invalid input", "USERS must be an integer.")

tests/test_in_progress.py

@@ -32,8 +32,9 @@ def test_run_in_progress_tests(
     actual_response = eligibility_client.make_request(
         nhs_number, headers=request_headers, query_params=query_params, strict_ssl=False
     )
-    expected_response = all_expected_responses.get(filename).get("response_items", {})
-
+    expected_response = all_expected_responses.get(filename, {}).get(
+        "response_items", {}
+    )
     expected_response_code = expected_response_code or http.HTTPStatus.OK
 
     assert actual_response["status_code"] == expected_response_code

tests/test_story_tests.py

@@ -36,7 +36,10 @@ def test_run_story_test_cases(
         query_params=query_params,
         strict_ssl=False,
     )
-    expected_response = all_expected_responses.get(filename).get("response_items", {})
+
+    expected_response = all_expected_responses.get(filename, {}).get(
+        "response_items", {}
+    )
     expected_response_code = expected_response_code or http.HTTPStatus.OK
 
     assert actual_response["status_code"] == expected_response_code

utils/common_utils.py

@@ -1,16 +1,49 @@
+import logging
 from pathlib import Path
 
+logger = logging.getLogger(__name__)
 
-def save_to_file(file_name: str, data, directory: str = None):
-    if directory is not None:
-        Path.mkdir(Path(directory), parents=True, exist_ok=True)
-    else:
-        directory = Path.cwd()
 
-    with Path.open(Path(directory) / file_name, "w") as f:
-        f.write(data)
+def save_to_file(file_name: str, data: str, directory: str | None = None) -> None:
+    """Write ``data`` to ``directory/file_name``, creating the directory if needed.
 
+    Args:
+        file_name: Name of the file to write.
+        data: String content to write.
+        directory: Optional directory path. Defaults to the current working directory.
 
-def load_from_file(file_name):
-    with Path.open(file_name, "r") as f:
-        return f.read()
+    Raises:
+        OSError: If the file cannot be written.
+    """
+    target_dir = Path(directory) if directory is not None else Path.cwd()
+    try:
+        target_dir.mkdir(parents=True, exist_ok=True)
+        with (target_dir / file_name).open("w", encoding="utf-8") as f:
+            f.write(data)
+    except (OSError, IOError) as e:
+        logger.error("Failed to save file %s to %s: %s", file_name, target_dir, e)
+        raise
+
+
+def load_from_file(file_name: str) -> str:
+    """Read and return the full text content of ``file_name``.
+
+    Args:
+        file_name: Path to the file to read.
+
+    Returns:
+        The file contents as a string.
+
+    Raises:
+        FileNotFoundError: If the file does not exist.
+        OSError: If the file cannot be read.
+    """
+    try:
+        with Path(file_name).open("r", encoding="utf-8") as f:
+            return f.read()
+    except FileNotFoundError:
+        logger.error("File not found: %s", file_name)
+        raise
+    except (OSError, IOError) as e:
+        logger.error("Failed to read file %s: %s", file_name, e)
+        raise