Refactoring focused on TerraformCommand

The work to continue refactoring this code continues; the primary motiviation is to improve unit tests, and lower the friction to adding new functionality. The TerraformCommand is a focus for this effort due to both the number of largely untestable methods, and its criticality to the project as a whole. - centralize many exceptions into the tfworker.exceptions module - create tfworker.types to start using enums for common strings - JSON type moved out of an __init__ into types - create TerraformAction type (plan/apply/destroy) - create TerraformStage type (pre/post, relation to Action) - create a tfworker.util.terraform for terraform utility methods - move get_terraform_version to tfworker.util.terraform - move get_platform to tfworker.util.system - create tfworker.util.hooks to handle all operations related to executing hook scripts. These all forced onto the TerraformCommand class previously - hooks has high test coverage - move - clean up and create constants for references to files that tfworker creates - In TerraformCommand: - break down the logic that determines how to plan into smaller methods - reorder and group remaining methods - removed all the hook related methods There are a lot of changes overall to the tests which are not enumerated, the complexity of testing is going down, and coverage is going up. Any areas of touched code are standardizing on using the Google Python Docstring comments format, and are having type annotations added
ephur · Jun 15, 2024 · 592e7fa · 592e7fa
1 parent 8de8f35
commit 592e7fa
Show file tree

Hide file tree

Showing 21 changed files with 1,143 additions and 514 deletions.
diff --git a/tests/commands/test_root.py b/tests/commands/test_root.py
@@ -16,13 +16,12 @@
 import os
 import platform
 from tempfile import TemporaryDirectory
-from unittest import mock
 
 import pytest
 from deepdiff import DeepDiff
 
 import tfworker.commands.root
-from tfworker.commands.root import get_platform, ordered_config_load
+from tfworker.commands.root import ordered_config_load
 
 
 class TestMain:
@@ -220,32 +219,6 @@ def test_config_formats(self, yaml_base_rootc, json_base_rootc, hcl_base_rootc):
         diff = DeepDiff(json_config, hcl_config)
         assert len(diff) == 0
 
-    @pytest.mark.parametrize(
-        "opsys, machine, mock_platform_opsys, mock_platform_machine",
-        [
-            ("linux", "i386", ["linux2"], ["i386"]),
-            ("linux", "arm", ["Linux"], ["arm"]),
-            ("linux", "amd64", ["linux"], ["x86_64"]),
-            ("linux", "amd64", ["linux"], ["amd64"]),
-            ("darwin", "amd64", ["darwin"], ["x86_64"]),
-            ("darwin", "amd64", ["darwin"], ["amd64"]),
-            ("darwin", "arm", ["darwin"], ["arm"]),
-            ("darwin", "arm64", ["darwin"], ["aarch64"]),
-        ],
-    )
-    def test_get_platform(
-        self, opsys, machine, mock_platform_opsys, mock_platform_machine
-    ):
-        with mock.patch("platform.system", side_effect=mock_platform_opsys) as mock1:
-            with mock.patch(
-                "platform.machine", side_effect=mock_platform_machine
-            ) as mock2:
-                actual_opsys, actual_machine = get_platform()
-                assert opsys == actual_opsys
-                assert machine == actual_machine
-                mock1.assert_called_once()
-                mock2.assert_called_once()
-
 
 class TestOrderedConfigLoad:
     def test_ordered_config_load(self):

diff --git a/tests/commands/test_terraform.py b/tests/commands/test_terraform.py
@@ -22,7 +22,7 @@
 from google.cloud.exceptions import NotFound
 
 import tfworker
-from tfworker.commands.terraform import BaseCommand, TerraformCommand, TerraformError
+from tfworker.commands.terraform import TerraformCommand, TerraformError
 from tfworker.definitions import Definition
 from tfworker.handlers import HandlerError
 
@@ -185,27 +185,6 @@ def test_run(self, tf_cmd: str, method: callable, args: list, request):
             for arg in args:
                 assert arg in call_as_string
 
-    @pytest.mark.parametrize(
-        "stdout, major, minor, expected_exception",
-        [
-            ("Terraform v0.12.29", 0, 12, does_not_raise()),
-            ("Terraform v1.3.5", 1, 3, does_not_raise()),
-            ("TF 14", "", "", pytest.raises(SystemExit)),
-        ],
-    )
-    def test_get_tf_version(
-        self, stdout: str, major: int, minor: int, expected_exception: callable
-    ):
-        with mock.patch(
-            "tfworker.commands.base.pipe_exec",
-            side_effect=mock_tf_version,
-        ) as mocked:
-            with expected_exception:
-                (actual_major, actual_minor) = BaseCommand.get_terraform_version(stdout)
-                assert actual_major == major
-                assert actual_minor == minor
-                mocked.assert_called_once()
-
     def test_worker_options(self, tf_13cmd_options):
         # Verify that the options from the CLI override the options from the config
         assert tf_13cmd_options._rootc.worker_options_odict.get("backend") == "s3"
@@ -236,7 +215,7 @@ def test_worker_options(self, tf_13cmd_options):
     def test_no_create_backend_bucket_fails_gcs(self, grootc_no_create_backend_bucket):
         with pytest.raises(SystemExit):
             with mock.patch(
-                "tfworker.commands.base.BaseCommand.get_terraform_version",
+                "tfworker.commands.base.get_terraform_version",
                 side_effect=lambda x: (13, 3),
             ):
                 with mock.patch(
@@ -367,7 +346,7 @@ def test_exec_valid_flow(self, terraform_command, definition):
             mock_prep_modules.assert_called_once_with(
                 terraform_command._terraform_modules_dir,
                 terraform_command._temp_dir,
-                required=False,
+                required=True,
             )
             mock_prep_and_init.assert_called_once_with(def_iter)
             mock_check_plan.assert_called_once_with(definition)
@@ -414,7 +393,7 @@ def test_exec_without_plan(self, terraform_command, definition):
             mock_prep_modules.assert_called_once_with(
                 terraform_command._terraform_modules_dir,
                 terraform_command._temp_dir,
-                required=False,
+                required=True,
             )
             mock_prep_and_init.assert_called_once_with(def_iter)
             mock_check_plan.assert_called_once_with(definition)
@@ -449,7 +428,7 @@ def test_exec_with_no_apply_or_destroy(self, terraform_command, definition):
             mock_prep_modules.assert_called_once_with(
                 terraform_command._terraform_modules_dir,
                 terraform_command._temp_dir,
-                required=False,
+                required=True,
             )
             mock_prep_and_init.assert_called_once_with(def_iter)
             mock_check_plan.assert_called_once_with(definition)

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -304,7 +304,7 @@ def rootc_options(s3_client, dynamodb_client, sts_client):
 @pytest.fixture
 def basec(rootc, s3_client):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.commands.base.get_terraform_version",
         side_effect=lambda x: (13, 3),
     ):
         with mock.patch(
@@ -319,7 +319,7 @@ def basec(rootc, s3_client):
 @pytest.fixture
 def gbasec(grootc):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.commands.base.get_terraform_version",
         side_effect=lambda x: (13, 3),
     ):
         with mock.patch(
@@ -342,7 +342,7 @@ def tf_Xcmd(rootc):
 @pytest.fixture
 def tf_15cmd(rootc):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.util.terraform.get_terraform_version",
         side_effect=lambda x: (15, 0),
     ):
         with mock.patch(
@@ -357,7 +357,7 @@ def tf_15cmd(rootc):
 @pytest.fixture
 def tf_14cmd(rootc):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.util.terraform.get_terraform_version",
         side_effect=lambda x: (14, 5),
     ):
         with mock.patch(
@@ -372,7 +372,7 @@ def tf_14cmd(rootc):
 @pytest.fixture
 def tf_13cmd(rootc):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.util.terraform.get_terraform_version",
         side_effect=lambda x: (13, 5),
     ):
         with mock.patch(
@@ -387,7 +387,7 @@ def tf_13cmd(rootc):
 @pytest.fixture
 def tf_12cmd(rootc):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.util.terraform.get_terraform_version",
         side_effect=lambda x: (12, 27),
     ):
         with mock.patch(
@@ -402,7 +402,7 @@ def tf_12cmd(rootc):
 @pytest.fixture
 def tf_13cmd_options(rootc_options):
     with mock.patch(
-        "tfworker.commands.base.BaseCommand.get_terraform_version",
+        "tfworker.util.terraform.get_terraform_version",
         side_effect=lambda x: (13, 5),
     ):
         with mock.patch(

diff --git a/tests/test_plugins.py b/tests/test_plugins.py
@@ -20,9 +20,10 @@
 
 import tfworker.commands.root
 import tfworker.plugins
+from tfworker.util.system import get_platform
 
 # values needed by multiple tests
-opsys, machine = tfworker.commands.root.get_platform()
+opsys, machine = get_platform()
 _platform = f"{opsys}_{machine}"