Feature: job to manage environment variables now handles remove act…

…ion (#199)

docs/jobs/environment_variables.md

@@ -10,12 +10,13 @@ Sample job configuration in your scenario file:
 
 ```yaml
 - name: Set environment variables
-    uses: manage-env-vars
-    with:
-        - QGIS_GLOBAL_SETTINGS_FILE:  # the name of environment variable
-            action: add
-            scope: user
-            value: "\\SIG\\QGIS\\CONFIG\\qgis_global_settings.ini"
+  uses: manage-env-vars
+  with:
+    - name: QGIS_GLOBAL_SETTINGS_FILE
+      action: "add"
+      value: "~/scripts/qgis_startup.py"
+      value: "\\SIG\\QGIS\\CONFIG\\qgis_global_settings.ini"
+      scope: "user"
 ```
 
 ----
@@ -31,6 +32,10 @@ Possible_values:
 - `add`: add environment variable
 - `remove`: remove environment variable
 
+### name
+
+Name of the environment variable.
+
 ### scope
 
 Level of the environment variable.

qgis_deployment_toolbelt/jobs/job_environment_variables.py

@@ -20,6 +20,7 @@
 
 # package
 from qgis_deployment_toolbelt.utils.win32utils import (
+    delete_environment_variable,
     refresh_environment,
     set_environment_variable,
 )
@@ -42,16 +43,45 @@ class JobEnvironmentVariables:
     """
 
     ID: str = "manage-env-vars"
+    OPTIONS_SCHEMA: dict = {
+        "action": {
+            "type": str,
+            "required": False,
+            "default": "add",
+            "possible_values": ("add", "remove"),
+            "condition": "in",
+        },
+        "name": {
+            "type": str,
+            "required": True,
+            "default": None,
+            "possible_values": None,
+            "condition": None,
+        },
+        "scope": {
+            "type": str,
+            "required": False,
+            "default": "user",
+            "possible_values": ("system", "user"),
+            "condition": "in",
+        },
+        "value": {
+            "type": (bool, int, str, list),
+            "required": False,
+            "default": None,
+            "possible_values": None,
+            "condition": None,
+        },
+    }
 
     def __init__(self, options: List[dict]) -> None:
         """Instantiate the class.
 
         Args:
-            options (List[dict]): dictionary with environment variable name as key and
-        some parameters as values (value, scope, action...).
+            options (List[dict]): list of dictionary with environment variables to set
+            or remove.
         """
-
-        self.options: dict = self.validate_options(options)
+        self.options: List[dict] = [self.validate_options(opt) for opt in options]
 
     def run(self) -> None:
         """Apply environment variables from dictionary to the system."""
@@ -68,6 +98,16 @@ def run(self) -> None:
                         logger.debug(
                             f"Variable name '{env_var.get('name')}' is not defined"
                         )
+                elif env_var.get("action") == "remove":
+                    try:
+                        delete_environment_variable(
+                            envvar_name=env_var.get("name"),
+                            scope=env_var.get("scope"),
+                        )
+                    except NameError:
+                        logger.debug(
+                            f"Variable name '{env_var.get('name')}' is not defined"
+                        )
             # force Windows to refresh the environment
             refresh_environment()
 
@@ -99,17 +139,53 @@ def prepare_value(self, value: str) -> str:
 
         return str(value).strip()
 
-    def validate_options(self, options: List[dict]) -> List[dict]:
+    # -- INTERNAL LOGIC ------------------------------------------------------
+    def validate_options(self, options: dict) -> dict:
         """Validate options.
 
-        :param List[dict] options: options to validate.
-        :return List[dict]: options if they are valid.
+        Args:
+            options (dict): options to validate.
+
+        Raises:
+            ValueError: if option has an invalid name or doesn't comply with condition
+            TypeError: if the option does'nt not comply with expected type
+
+        Returns:
+            dict: options if valid
         """
-        if not isinstance(options, list):
-            raise TypeError(f"Options must be a list, not {type(options)}")
         for option in options:
-            if not isinstance(option, dict):
-                raise TypeError(f"Options must be a dict, not {type(option)}")
+            if option not in self.OPTIONS_SCHEMA:
+                raise ValueError(
+                    f"Job: {self.ID}. Option '{option}' is not valid."
+                    f" Valid options are: {self.OPTIONS_SCHEMA.keys()}"
+                )
+
+            option_in = options.get(option)
+            option_def: dict = self.OPTIONS_SCHEMA.get(option)
+            # check value type
+            if not isinstance(option_in, option_def.get("type")):
+                raise TypeError(
+                    f"Job: {self.ID}. Option '{option}' has an invalid value."
+                    f"\nExpected {option_def.get('type')}, got {type(option_in)}"
+                )
+            # check value condition
+            if option_def.get("condition") == "startswith" and not option_in.startswith(
+                option_def.get("possible_values")
+            ):
+                raise ValueError(
+                    f"Job: {self.ID}. Option '{option}' has an invalid value."
+                    "\nExpected: starts with one of: "
+                    f"{', '.join(option_def.get('possible_values'))}"
+                )
+            elif option_def.get(
+                "condition"
+            ) == "in" and option_in not in option_def.get("possible_values"):
+                raise ValueError(
+                    f"Job: {self.ID}. Option '{option}' has an invalid value."
+                    f"\nExpected: one of: {', '.join(option_def.get('possible_values'))}"
+                )
+            else:
+                pass
 
         return options
 

qgis_deployment_toolbelt/utils/win32utils.py

@@ -65,7 +65,14 @@ def delete_environment_variable(envvar_name: str, scope: str = "user") -> bool:
         hkey = user_hkey
     else:
         system_hkey
-    # try to get the value
+
+    # get it to check if variable exits
+    try:
+        get_environment_variable(envvar_name=envvar_name, scope=scope)
+    except Exception:
+        return False
+
+    # try to delete the variable
     try:
         with winreg.OpenKey(*hkey, access=winreg.KEY_ALL_ACCESS) as key:
             winreg.DeleteValue(key, envvar_name)

tests/fixtures/scenarios/good_scenario_sample.qdt.yml

@@ -21,10 +21,14 @@ steps:
   - name: Set environment variables
     uses: manage-env-vars
     with:
-      - PYQGIS_STARTUP:
+      - name: PYQGIS_STARTUP
+        action: "add"
         value: "~/scripts/qgis_startup.py"
         scope: "user"
+      - name: QDT_TEST_FAKE_ENV_VAR_BOOL
         action: "add"
+        value: true
+        scope: "user"
 
   - name: Synchronize QGIS profiles from remote location
     uses: qprofiles-manager

tests/test_job_environment_variables.py

@@ -63,7 +63,7 @@ def tearDown(self):
 
     # -- TESTS ---------------------------------------------------------
     @unittest.skipIf(opersys != "win32", "Test specific to Windows.")
-    def test_environment_variables_set(self):
+    def test_environment_variables_set_unset(self):
         """Test YAML loader"""
         fake_env_vars = [
             {
@@ -95,6 +95,29 @@ def test_environment_variables_set(self):
                 str(Path(expanduser("~/scripts/qgis_startup.py")).resolve()),
             )
 
+            # clean up
+            fake_env_vars = [
+                {
+                    "name": "QDT_TEST_FAKE_ENV_VAR_BOOL",
+                    "scope": "user",
+                    "action": "remove",
+                },
+                {
+                    "name": "QDT_TEST_FAKE_ENV_VAR_PATH",
+                    "scope": "user",
+                    "action": "remove",
+                },
+            ]
+            job_env_vars = JobEnvironmentVariables(fake_env_vars)
+            job_env_vars.run()
+
+            self.assertIsNone(
+                get_environment_variable("QDT_TEST_FAKE_ENV_VAR_BOOL", "user")
+            )
+            self.assertIsNone(
+                get_environment_variable("QDT_TEST_FAKE_ENV_VAR_PATH"),
+            )
+
     def test_prepare_value(self):
         """Test prepare_value method"""
         job_env_vars = JobEnvironmentVariables([])
@@ -126,8 +149,28 @@ def test_prepare_value(self):
     def test_validate_options(self):
         """Test validate_options method"""
         job_env_vars = JobEnvironmentVariables([])
-        # Options must be a list of dictionaries
-        with self.assertRaises(TypeError):
+        # Options must be a dictionary
+        with self.assertRaises(ValueError):
             job_env_vars.validate_options(options="options_test")
-        with self.assertRaises(TypeError):
+        with self.assertRaises(ValueError):
             job_env_vars.validate_options(options=["options_test"])
+
+        bad_options_scope = [
+            {
+                "action": "remove",
+                "name": "QDT_TEST_FAKE_ENV_VAR_BOOL",
+                "scope": "imaginary_scope",
+            }
+        ]
+        bad_options_action = [
+            {
+                "action": "update",
+                "name": "QDT_TEST_FAKE_ENV_VAR_PATH",
+                "scope": "user",
+            },
+        ]
+
+        with self.assertRaises(ValueError):
+            JobEnvironmentVariables(bad_options_action)
+        with self.assertRaises(ValueError):
+            JobEnvironmentVariables(bad_options_scope)