change: add unit tests and remove unreachable condition

Author: benieric

src/sagemaker/modules/train/model_trainer.py

@@ -483,8 +483,6 @@ def model_post_init(self, __context: Any):
                 except json.JSONDecodeError:
                     try:
                         self.hyperparameters = yaml.safe_load(contents)
-                        if not isinstance(self.hyperparameters, dict):
-                            raise ValueError("YAML content is not a valid mapping.")
                         logger.debug("Hyperparameters loaded as YAML")
                     except (yaml.YAMLError, ValueError):
                         raise ValueError(

tests/unit/sagemaker/modules/train/test_model_trainer.py

@@ -17,9 +17,10 @@
 import tempfile
 import json
 import os
+import yaml
 import pytest
 from pydantic import ValidationError
-from unittest.mock import patch, MagicMock, ANY
+from unittest.mock import patch, MagicMock, ANY, mock_open
 
 from sagemaker import image_uris
 from sagemaker_core.main.resources import TrainingJob
@@ -1093,3 +1094,68 @@ def test_destructor_cleanup(mock_tmp_dir, modules_session):
     mock_tmp_dir.assert_not_called()
     del model_trainer
     mock_tmp_dir.cleanup.assert_called_once()
+
+
+@patch("os.path.exists")
+def test_hyperparameters_valid_json(mock_exists, modules_session):
+    mock_exists.return_value = True
+    expected_hyperparameters = {"param1": "value1", "param2": 2}
+    mock_file_open = mock_open(read_data=json.dumps(expected_hyperparameters))
+
+    with patch("builtins.open", mock_file_open):
+        model_trainer = ModelTrainer(
+            training_image=DEFAULT_IMAGE,
+            role=DEFAULT_ROLE,
+            sagemaker_session=modules_session,
+            compute=DEFAULT_COMPUTE_CONFIG,
+            hyperparameters="hyperparameters.json",
+        )
+        assert model_trainer.hyperparameters == expected_hyperparameters
+        mock_file_open.assert_called_once_with("hyperparameters.json", "r")
+        mock_exists.assert_called_once_with("hyperparameters.json")
+
+
+@patch("os.path.exists")
+def test_hyperparameters_valid_yaml(mock_exists, modules_session):
+    mock_exists.return_value = True
+    expected_hyperparameters = {"param1": "value1", "param2": 2}
+    mock_file_open = mock_open(read_data=yaml.dump(expected_hyperparameters))
+
+    with patch("builtins.open", mock_file_open):
+        model_trainer = ModelTrainer(
+            training_image=DEFAULT_IMAGE,
+            role=DEFAULT_ROLE,
+            sagemaker_session=modules_session,
+            compute=DEFAULT_COMPUTE_CONFIG,
+            hyperparameters="hyperparameters.yaml",
+        )
+        assert model_trainer.hyperparameters == expected_hyperparameters
+        mock_file_open.assert_called_once_with("hyperparameters.yaml", "r")
+        mock_exists.assert_called_once_with("hyperparameters.yaml")
+
+
+def test_hyperparameters_not_exist(modules_session):
+    with pytest.raises(ValueError):
+        ModelTrainer(
+            training_image=DEFAULT_IMAGE,
+            role=DEFAULT_ROLE,
+            sagemaker_session=modules_session,
+            compute=DEFAULT_COMPUTE_CONFIG,
+            hyperparameters="nonexistent.json",
+        )
+
+
+@patch("os.path.exists")
+def test_hyperparameters_invalid(mock_exists, modules_session):
+    mock_exists.return_value = True
+    # Must be valid YAML or JSON
+    mock_file_open = mock_open(read_data="invalid")
+    with patch("builtins.open", mock_file_open):
+        with pytest.raises(ValueError, match="Must be a valid JSON or YAML file."):
+            ModelTrainer(
+                training_image=DEFAULT_IMAGE,
+                role=DEFAULT_ROLE,
+                sagemaker_session=modules_session,
+                compute=DEFAULT_COMPUTE_CONFIG,
+                hyperparameters="hyperparameters.yaml",
+            )