Fix copilot review feedback on functional tests

tests/test_functional.py

@@ -3,7 +3,6 @@
 Tests cover:
 - hook.py syntax validation via ast.parse
 - Abilities YAML validation (if present)
-- Requirements.txt dependency checks (if present)
 - Security pattern scanning across Python source files
 """
 import ast
@@ -22,15 +21,15 @@ class TestHookSyntax:
     def test_hook_parses(self):
         """hook.py should be valid Python syntax."""
         hook_path = os.path.join(PLUGIN_DIR, "hook.py")
-        with open(hook_path, "r") as fh:
+        with open(hook_path, "r", encoding="utf-8") as fh:
             source = fh.read()
         tree = ast.parse(source, filename="hook.py")
         assert isinstance(tree, ast.Module)
 
     def test_hook_has_no_bare_exec(self):
         """hook.py should not contain bare exec() calls."""
         hook_path = os.path.join(PLUGIN_DIR, "hook.py")
-        with open(hook_path, "r") as fh:
+        with open(hook_path, "r", encoding="utf-8") as fh:
             source = fh.read()
         tree = ast.parse(source)
         for node in ast.walk(tree):
@@ -47,7 +46,7 @@ def test_all_py_files_parse(self):
                 if not fname.endswith(".py"):
                     continue
                 fpath = os.path.join(root, fname)
-                with open(fpath, "r") as fh:
+                with open(fpath, "r", encoding="utf-8") as fh:
                     source = fh.read()
                 try:
                     ast.parse(source, filename=fname)
@@ -66,17 +65,22 @@ def _yaml_files():
 
     def test_abilities_directory_exists(self):
         """data/abilities/ directory should exist."""
-        assert os.path.isdir(os.path.join(PLUGIN_DIR, "data", "abilities"))
+        abilities_dir = os.path.join(PLUGIN_DIR, "data", "abilities")
+        if not os.path.isdir(abilities_dir):
+            pytest.skip("data/abilities/ directory not found")
+        assert os.path.isdir(abilities_dir)
 
     def test_abilities_yaml_files_exist(self):
         """There should be at least one YAML ability file."""
-        assert len(self._yaml_files()) > 0, "No .yml files found in data/abilities/"
+        if not self._yaml_files():
+            pytest.skip("No .yml files found in data/abilities/")
+        assert len(self._yaml_files()) > 0
 
     def test_abilities_yaml_parseable(self):
         """Each abilities YAML file should be parseable."""
-        import yaml
+        yaml = pytest.importorskip("yaml")
         for yf in self._yaml_files():
-            with open(yf, "r") as fh:
+            with open(yf, "r", encoding="utf-8") as fh:
                 try:
                     docs = list(yaml.safe_load_all(fh))
                 except yaml.YAMLError as exc:
@@ -85,9 +89,9 @@ def test_abilities_yaml_parseable(self):
 
     def test_abilities_have_required_fields(self):
         """Each ability must have id, name, and tactic fields."""
-        import yaml
+        yaml = pytest.importorskip("yaml")
         for yf in self._yaml_files():
-            with open(yf, "r") as fh:
+            with open(yf, "r", encoding="utf-8") as fh:
                 docs = list(yaml.safe_load_all(fh))
             for doc in docs:
                 if doc is None:
@@ -103,10 +107,10 @@ def test_abilities_have_required_fields(self):
 
     def test_abilities_ids_are_unique(self):
         """Ability IDs should not be duplicated within the plugin."""
-        import yaml
+        yaml = pytest.importorskip("yaml")
         seen = {}
         for yf in self._yaml_files():
-            with open(yf, "r") as fh:
+            with open(yf, "r", encoding="utf-8") as fh:
                 docs = list(yaml.safe_load_all(fh))
             for doc in docs:
                 if doc is None:
@@ -138,7 +142,7 @@ def _py_files():
     def test_no_verify_false(self):
         """No Python file should use verify=False (disables TLS verification)."""
         for fpath in self._py_files():
-            with open(fpath, "r") as fh:
+            with open(fpath, "r", encoding="utf-8") as fh:
                 for lineno, line in enumerate(fh, 1):
                     if "verify=False" in line and not line.strip().startswith("#"):
                         rel = os.path.relpath(fpath, PLUGIN_DIR)
@@ -153,7 +157,7 @@ def test_no_unguarded_shell_true(self):
             fname = os.path.basename(fpath)
             if any(fname.startswith(a) or fname == a for a in allowlist):
                 continue
-            with open(fpath, "r") as fh:
+            with open(fpath, "r", encoding="utf-8") as fh:
                 for lineno, line in enumerate(fh, 1):
                     stripped = line.strip()
                     if stripped.startswith("#"):
@@ -168,13 +172,13 @@ def test_requests_have_timeout(self):
         """requests.get/post/put/delete calls should include a timeout parameter."""
         pattern = re.compile(r"requests\.(get|post|put|delete|patch|head)\(")
         for fpath in self._py_files():
-            with open(fpath, "r") as fh:
+            with open(fpath, "r", encoding="utf-8") as fh:
                 source = fh.read()
             for match in pattern.finditer(source):
                 start = match.start()
                 depth = 0
                 end = start
-                for i in range(start, min(start + 500, len(source))):
+                for i in range(start, len(source)):
                     if source[i] == "(":
                         depth += 1
                     elif source[i] == ")":