Merge pull request #94 from jakeberesford-palmetto/bugfix/plugin-conf…

…ig-bug

[BUGFIX] plugin.update returning boolean, needs config object.

palm/plugins/base_plugin_config.py

@@ -1,7 +1,8 @@
-from abc import ABC, abstractmethod
 import click
-from pathlib import Path
 import yaml
+from typing import Tuple
+from abc import ABC, abstractmethod
+from pathlib import Path
 from pydantic import BaseModel, ValidationError
 
 from palm.palm_exceptions import InvalidConfigError
@@ -26,23 +27,24 @@ def set(self) -> dict:
         """
         pass
 
-    def update(self) -> bool:
+    def update(self) -> Tuple[bool, dict]:
         """Updates the config file with configuration provided by the set method
 
         Returns:
-            bool: True if the config was updated, False if it was not
+            Tuple[bool, dict]: Returns a tuple of (True, config) if the config was
+            updated successfully, and (False, {}) if it was not.
         """
         try:
             config = self.set()
             self.validate(config)
             self._write(config)
         except InvalidConfigError as e:
             click.secho(str(e), fg="red")
-            return False
+            raise e
         except Exception as e:
             click.secho(str(e), fg="red")
-            return False
-        return True
+            return (False, {})
+        return (True, config)
 
     def get(self) -> BaseModel:
         if not self.config_path.exists():
@@ -75,10 +77,7 @@ def _read(self) -> dict:
         plugin_config = palm_config.get('plugin_config', {}).get(self.plugin_name, {})
 
         if not plugin_config:
-            plugin_config = self.update()
-
-        if not self.validate(plugin_config):
-            raise InvalidConfigError
+            _, plugin_config = self.update()
 
         return plugin_config
 

palm/plugins/core/commands/cmd_plugin.py

@@ -134,7 +134,7 @@ def configure(environment, name: Optional[str]):
         click.secho(f"Plugin {name} not installed in this project", fg="red")
         return
 
-    success = plugin.config.update()
+    success, _ = plugin.config.update()
     if success:
         click.secho(f"Plugin {plugin.name} configured successfully", fg="green")
     else:

tests/unit/test_plugin_config.py

@@ -1,5 +1,6 @@
 import pytest
 import yaml
+from unittest import mock
 
 from pathlib import Path
 from palm.palm_exceptions import InvalidConfigError
@@ -42,3 +43,65 @@ def test_get_config(mock_plugin_config, tmp_path, monkeypatch):
     monkeypatch.setattr(config, "config_path", config_path)
 
     assert config.get() == {"value": "foo"}
+
+
+@pytest.mark.parametrize("config", [{"value": "foo"}])
+def test_update_config_returns_success_and_config(
+    mock_plugin_config, tmp_path, monkeypatch
+):
+    config = mock_plugin_config
+    config_path = tmp_path / "config.yaml"
+    config_path.write_text(yaml.dump({'image_name': 'palm-test'}))
+    monkeypatch.setattr(config, "config_path", config_path)
+
+    assert config.update() == (True, {"value": "foo"})
+
+
+@pytest.mark.parametrize("config", [{"value": "foo"}])
+def test_update_config_writes_to_file(mock_plugin_config, tmp_path, monkeypatch):
+    config = mock_plugin_config
+    config_path = tmp_path / "config.yaml"
+    config_path.write_text(yaml.dump({'image_name': 'palm-test'}))
+    monkeypatch.setattr(config, "config_path", config_path)
+
+    config.update()
+
+    config = yaml.safe_load(config_path.read_text())
+    assert config["plugin_config"]["test"]["value"] == "foo"
+
+
+@pytest.mark.parametrize("config", [{}])
+def test_update_config_raises_for_invalid_config(
+    mock_plugin_config, tmp_path, monkeypatch
+):
+    config = mock_plugin_config
+    config_path = tmp_path / "config.yaml"
+    config_path.write_text(yaml.dump({'image_name': 'palm-test'}))
+    monkeypatch.setattr(config, "config_path", config_path)
+
+    with pytest.raises(InvalidConfigError):
+        config.update()
+
+
+@pytest.mark.parametrize("config", [{}])
+def test_read_config_with_update(mock_plugin_config, tmp_path, monkeypatch):
+    config = mock_plugin_config
+    config_path = tmp_path / "config.yaml"
+    config_path.write_text(yaml.dump({'image_name': 'palm-test'}))
+    monkeypatch.setattr(config, "config_path", config_path)
+    monkeypatch.setattr(config, "update", lambda: (True, {"value": "bar"}))
+
+    assert config._read() == {"value": "bar"}
+
+
+@pytest.mark.parametrize("config", [{}])
+def test_read_config_existing(mock_plugin_config, tmp_path, monkeypatch):
+    config = mock_plugin_config
+    config_path = tmp_path / "config.yaml"
+    config_path.write_text(yaml.dump({'plugin_config': {'test': {'value': 'foo'}}}))
+    monkeypatch.setattr(config, "config_path", config_path)
+    m = mock.Mock()
+    monkeypatch.setattr(config, "set", m)
+
+    assert config._read() == {"value": "foo"}
+    assert m.called == False