Fix bug with string task declarations in included task files (#159)

Fixes #158

Poe no longer crashes if a included task file is configured with `cwd` and includes string task declarations.

Included task files may now include the following global options which will be respected for associated tasks:
- default_task_type
- default_array_task_type
- default_array_item_task_type

---------

Co-authored-by: Nat Noordanus <[email protected]>

poethepoet/config.py

@@ -187,7 +187,7 @@ def validate(self):
         # Validate shell_interpreter type
         for interpreter in self.shell_interpreter:
             if interpreter not in self.KNOWN_SHELL_INTERPRETERS:
-                return (
+                raise PoeException(
                     f"Unsupported value {interpreter!r} for option `shell_interpreter`."
                 )
 
@@ -262,50 +262,53 @@ def _load_includes(self, project_dir: Path):
                 # TODO: print warning in verbose mode, requires access to ui somehow
                 continue
 
-            include_config = self._read_config_file(include_path)
-            self._merge_config(include_config, include_path, include.get("cwd"))
+            try:
+                include_config = PoeConfig(
+                    cwd=include.get("cwd", self.project_dir),
+                    table=self._read_config_file(include_path)["tool"]["poe"],
+                )
+                include_config._project_dir = self._project_dir
+                include_config.validate()
+            except (PoeException, KeyError) as error:
+                raise PoeException(
+                    f"Invalid content in included file from {include_path}", error
+                ) from error
 
-    def _merge_config(
-        self,
-        extra_config: Mapping[str, Any],
-        include_path: Path,
-        task_cwd: Optional[str],
-    ):
-        try:
-            poe_config = extra_config.get("tool", {}).get("poe", {})
-            tasks = poe_config.get("tasks", {})
-        except AttributeError as error:
-            raise PoeException(
-                f"Invalid content in included file from {include_path}", error
-            ) from error
+            self._merge_config(include_config)
+
+    def _merge_config(self, include_config: "PoeConfig"):
+        from .task import PoeTask
 
         # Env is special because it can be extended rather than just overwritten
-        if "env" in poe_config:
-            self._poe["env"] = {**poe_config["env"], **self._poe.get("env", {})}
+        if include_config.global_env:
+            self._poe["env"] = {**include_config.global_env, **self._poe.get("env", {})}
 
-        if "envfile" in poe_config and "envfile" not in self._poe:
-            self._poe["envfile"] = poe_config["envfile"]
+        if include_config.global_envfile and "envfile" not in self._poe:
+            self._poe["envfile"] = include_config.global_envfile
 
         # Includes additional tasks with preserved ordering
         self._poe["tasks"] = own_tasks = self._poe.get("tasks", {})
-        for task_name, task_def in tasks.items():
-            if task_cwd:
+        for task_name, task_def in include_config.tasks.items():
+            if task_name in own_tasks:
+                # don't override tasks from the base config
+                continue
+
+            task_def = PoeTask.normalize_task_def(task_def, include_config)
+            if include_config.cwd:
                 # Override the config of each task to use the include level cwd as a
                 # base for the task level cwd
                 if "cwd" in task_def:
                     # rebase the configured cwd onto the include level cwd
-                    task_def["cwd"] = (
-                        Path(self.project_dir)
-                        .joinpath(task_cwd)
+                    task_def["cwd"] = str(
+                        Path(include_config.cwd)
                         .resolve()
                         .joinpath(task_def["cwd"])
                         .relative_to(self.project_dir)
                     )
                 else:
-                    task_def["cwd"] = task_cwd
+                    task_def["cwd"] = str(include_config.cwd)
 
-            if task_name not in own_tasks:
-                own_tasks[task_name] = task_def
+            own_tasks[task_name] = task_def
 
     @staticmethod
     def _read_config_file(path: Path) -> Mapping[str, Any]:

poethepoet/task/base.py

@@ -134,7 +134,7 @@ def from_def(
             options["capture_stdout"] = capture_stdout
 
         if isinstance(task_def, (str, list)):
-            task_def = cls._normalize_task_def(
+            task_def = cls.normalize_task_def(
                 task_def, config, task_type=cls.__task_types[task_type]
             )
 
@@ -151,7 +151,7 @@ def from_def(
         )
 
     @classmethod
-    def _normalize_task_def(
+    def normalize_task_def(
         cls,
         task_def: TaskDef,
         config: "PoeConfig",
@@ -450,7 +450,7 @@ def validate_def(
         elif isinstance(task_def, list):
             task_type_key = config.default_array_task_type
             task_type = cls.__task_types[task_type_key]
-            normalized_task_def = cls._normalize_task_def(
+            normalized_task_def = cls.normalize_task_def(
                 task_def, config, task_type=task_type
             )
             if hasattr(task_type, "_validate_task_def"):

poethepoet/task/sequence.py

@@ -147,7 +147,7 @@ def _validate_task_def(
             else:
                 subtask_issue = cls.validate_def(
                     cls._subtask_name(task_name, index),
-                    cls._normalize_task_def(
+                    cls.normalize_task_def(
                         task_item,
                         config,
                         array_item=default_item_type or True,

tests/fixtures/monorepo_project/subproject_2/pyproject.toml

@@ -1,7 +1,10 @@
 
 tool.poe.include = ["extra_tasks.toml", { path = "../pyproject.toml" }]
+tool.poe.default_task_type = "expr"
+
+[tool.poe.tasks]
+add = "1 + 1"
 
 [tool.poe.tasks.get_cwd_2]
 interpreter = "python"
 shell       = "import os; print(os.getcwd())"
-

tests/test_includes.py

@@ -88,6 +88,7 @@ def test_monorepo_contains_only_expected_tasks(run_poe_subproc, projects):
         "CONFIGURED TASKS\n"
         "  get_cwd_0      \n"
         "  get_cwd_1      \n"
+        "  add            \n"
         "  get_cwd_2      \n\n\n"
     )
     assert result.stdout == ""
@@ -98,6 +99,7 @@ def test_monorepo_can_also_include_parent(run_poe_subproc, projects, is_windows)
     result = run_poe_subproc(cwd=projects["monorepo/subproject_2"])
     assert result.capture.endswith(
         "CONFIGURED TASKS\n"
+        "  add            \n"
         "  get_cwd_2      \n"
         "  extra_task     \n"
         "  get_cwd_0      \n\n\n"
@@ -109,12 +111,17 @@ def test_monorepo_can_also_include_parent(run_poe_subproc, projects, is_windows)
     assert result.capture == "Poe => import os; print(os.getcwd())\n"
     if is_windows:
         assert result.stdout.endswith(
-            "poethepoet\\tests\\fixtures\\monorepo_project\\subproject_2\n"
+            "\\tests\\fixtures\\monorepo_project\\subproject_2\n"
         )
     else:
-        assert result.stdout.endswith(
-            "poethepoet/tests/fixtures/monorepo_project/subproject_2\n"
-        )
+        assert result.stdout.endswith("/tests/fixtures/monorepo_project/subproject_2\n")
+    assert result.stderr == ""
+
+
+def test_set_default_task_type_with_include(run_poe_subproc, projects, is_windows):
+    result = run_poe_subproc("add", cwd=projects["monorepo/subproject_2"])
+    assert result.capture == "Poe => 1 + 1\n"
+    assert result.stdout == "2\n"
     assert result.stderr == ""
 
 
@@ -124,27 +131,25 @@ def test_monorepo_runs_each_task_with_expected_cwd(
     result = run_poe_subproc("get_cwd_0", project="monorepo")
     assert result.capture == "Poe => import os; print(os.getcwd())\n"
     if is_windows:
-        assert result.stdout.endswith("poethepoet\\tests\\fixtures\\monorepo_project\n")
+        assert result.stdout.endswith("\\tests\\fixtures\\monorepo_project\n")
     else:
-        assert result.stdout.endswith("poethepoet/tests/fixtures/monorepo_project\n")
+        assert result.stdout.endswith("/tests/fixtures/monorepo_project\n")
     assert result.stderr == ""
 
     result = run_poe_subproc("get_cwd_1", project="monorepo")
     assert result.capture == "Poe => import os; print(os.getcwd())\n"
     if is_windows:
-        assert result.stdout.endswith("poethepoet\\tests\\fixtures\\monorepo_project\n")
+        assert result.stdout.endswith("\\tests\\fixtures\\monorepo_project\n")
     else:
-        assert result.stdout.endswith("poethepoet/tests/fixtures/monorepo_project\n")
+        assert result.stdout.endswith("/tests/fixtures/monorepo_project\n")
     assert result.stderr == ""
 
     result = run_poe_subproc("get_cwd_2", project="monorepo")
     assert result.capture == "Poe => import os; print(os.getcwd())\n"
     if is_windows:
         assert result.stdout.endswith(
-            "poethepoet\\tests\\fixtures\\monorepo_project\\subproject_2\n"
+            "\\tests\\fixtures\\monorepo_project\\subproject_2\n"
         )
     else:
-        assert result.stdout.endswith(
-            "poethepoet/tests/fixtures/monorepo_project/subproject_2\n"
-        )
+        assert result.stdout.endswith("/tests/fixtures/monorepo_project/subproject_2\n")
     assert result.stderr == ""