From 005921ad916d09b902b5a867ca52ba219cfb9bd3 Mon Sep 17 00:00:00 2001 From: Dan Dye Date: Tue, 26 Aug 2025 23:01:13 -0400 Subject: [PATCH 1/4] Add Python import validation to CI pipeline - Add GitHub Actions workflow for PR testing - Create tests/check_imports.py to validate all Python files can be imported - Tests all files in server/gti, server/scc, server/secops, server/secops-soar - Catches import errors like "from typing import str" that break production - Pragmatic approach that tests actual runtime imports - Will fail CI with exit code 1 on any import errors --- .github/workflows/pr-tests.yml | 34 ++++++++++++++ tests/check_imports.py | 83 ++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 .github/workflows/pr-tests.yml create mode 100644 tests/check_imports.py diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml new file mode 100644 index 00000000..83937c3e --- /dev/null +++ b/.github/workflows/pr-tests.yml @@ -0,0 +1,34 @@ +name: Run Tests on PR + +on: + pull_request: + branches: + - main + +jobs: + test: + runs-on: ubuntu-latest + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: '3.x' + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install pytest + pip install -e ./server/gti + pip install -e ./server/scc + pip install -e ./server/secops + pip install -e ./server/secops-soar + + - name: Check Python syntax and imports + run: python tests/check_imports.py + + - name: Run tests + run: | + pytest tests/ diff --git a/tests/check_imports.py b/tests/check_imports.py new file mode 100644 index 00000000..c79bd1b8 --- /dev/null +++ b/tests/check_imports.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +""" +Import validation script for CI/CD pipeline. + +This script tests all Python files in server directories to catch import errors +and syntax issues that could break production deployments. +""" + +import os +import sys +import importlib.util + + +def test_python_file(filepath): + """ + Test if a Python file can be imported without errors. + + Args: + filepath: Path to the Python file to test + + Returns: + bool: True if import succeeds, False otherwise + """ + try: + spec = importlib.util.spec_from_file_location('temp_module', filepath) + if spec and spec.loader: + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return True + except Exception as e: + print(f'✗ {filepath}: {e}') + return False + return True + + +def find_python_files(directory): + """ + Recursively find all Python files in a directory. + + Args: + directory: Directory to search + + Returns: + list: List of Python file paths + """ + python_files = [] + for root, dirs, files in os.walk(directory): + for file in files: + if file.endswith('.py'): + python_files.append(os.path.join(root, file)) + return python_files + + +def main(): + """Main function to test all Python files in server directories.""" + # Test all Python files in server directories + server_dirs = ['server/gti', 'server/scc', 'server/secops', 'server/secops-soar'] + failed_files = [] + total_files = 0 + + for server_dir in server_dirs: + if os.path.exists(server_dir): + python_files = find_python_files(server_dir) + total_files += len(python_files) + print(f'Testing {len(python_files)} Python files in {server_dir}...') + + for py_file in python_files: + if not test_python_file(py_file): + failed_files.append(py_file) + + print(f'\nTested {total_files} Python files total') + if failed_files: + print(f'\n{len(failed_files)} files failed import tests:') + for failed_file in failed_files: + print(f' - {failed_file}') + return 1 + else: + print('✓ All Python files import successfully') + return 0 + + +if __name__ == "__main__": + sys.exit(main()) \ No newline at end of file From ed1e6122530408a33ab080a6fb103f99faf58ce4 Mon Sep 17 00:00:00 2001 From: Dan Dye Date: Tue, 26 Aug 2025 23:14:31 -0400 Subject: [PATCH 2/4] Exclude setup.py files from import testing setup.py files execute setuptools commands when imported, causing false positive failures in CI. These files don't need import testing as they're not imported during normal runtime. --- tests/check_imports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/check_imports.py b/tests/check_imports.py index c79bd1b8..0c05c08c 100644 --- a/tests/check_imports.py +++ b/tests/check_imports.py @@ -46,7 +46,7 @@ def find_python_files(directory): python_files = [] for root, dirs, files in os.walk(directory): for file in files: - if file.endswith('.py'): + if file.endswith('.py') and file != 'setup.py': python_files.append(os.path.join(root, file)) return python_files From 807941f01ed338028a9d0d0d4be11a0c5c69eda1 Mon Sep 17 00:00:00 2001 From: Dan Dye Date: Tue, 26 Aug 2025 23:27:55 -0400 Subject: [PATCH 3/4] Fix import validation and code style - Fix models.py: remove invalid 'str' import from typing - Use double quotes consistently throughout check_imports.py - Exclude problematic file types from import testing: - setup.py files (execute setuptools commands when imported) - test directories and conftest.py (test fixtures) - example.py files (standalone examples) - __init__.py files (some have special import logic) - tools/ subdirectories (use relative imports) These exclusions prevent false positives while still catching real import errors in the main codebase. --- .../secops_soar_mcp/utils/models.py | 2 +- tests/check_imports.py | 25 +++++++---- tests/test_imports.py | 43 +++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 tests/test_imports.py diff --git a/server/secops-soar/secops_soar_mcp/utils/models.py b/server/secops-soar/secops_soar_mcp/utils/models.py index 0b260da6..3c589407 100644 --- a/server/secops-soar/secops_soar_mcp/utils/models.py +++ b/server/secops-soar/secops_soar_mcp/utils/models.py @@ -13,7 +13,7 @@ # limitations under the License. from enum import StrEnum from pydantic import BaseModel, Field -from typing import List, Dict, str, Optional, Any +from typing import List, Dict, Optional, Any class CasePriority(StrEnum): diff --git a/tests/check_imports.py b/tests/check_imports.py index 0c05c08c..1374748b 100644 --- a/tests/check_imports.py +++ b/tests/check_imports.py @@ -22,13 +22,13 @@ def test_python_file(filepath): bool: True if import succeeds, False otherwise """ try: - spec = importlib.util.spec_from_file_location('temp_module', filepath) + spec = importlib.util.spec_from_file_location("temp_module", filepath) if spec and spec.loader: module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) return True except Exception as e: - print(f'✗ {filepath}: {e}') + print(f"✗ {filepath}: {e}") return False return True @@ -43,10 +43,17 @@ def find_python_files(directory): Returns: list: List of Python file paths """ + # Files to skip + skip_files = {"setup.py", "example.py", "conftest.py", "__init__.py"} + python_files = [] for root, dirs, files in os.walk(directory): + # Skip test directories and tools subdirectories with relative imports + if "tests" in root or "__pycache__" in root or "/tools" in root: + continue + for file in files: - if file.endswith('.py') and file != 'setup.py': + if file.endswith(".py") and file not in skip_files: python_files.append(os.path.join(root, file)) return python_files @@ -54,7 +61,7 @@ def find_python_files(directory): def main(): """Main function to test all Python files in server directories.""" # Test all Python files in server directories - server_dirs = ['server/gti', 'server/scc', 'server/secops', 'server/secops-soar'] + server_dirs = ["server/gti", "server/scc", "server/secops", "server/secops-soar"] failed_files = [] total_files = 0 @@ -62,20 +69,20 @@ def main(): if os.path.exists(server_dir): python_files = find_python_files(server_dir) total_files += len(python_files) - print(f'Testing {len(python_files)} Python files in {server_dir}...') + print(f"Testing {len(python_files)} Python files in {server_dir}...") for py_file in python_files: if not test_python_file(py_file): failed_files.append(py_file) - print(f'\nTested {total_files} Python files total') + print(f"\nTested {total_files} Python files total") if failed_files: - print(f'\n{len(failed_files)} files failed import tests:') + print(f"\n{len(failed_files)} files failed import tests:") for failed_file in failed_files: - print(f' - {failed_file}') + print(f" - {failed_file}") return 1 else: - print('✓ All Python files import successfully') + print("✓ All Python files import successfully") return 0 diff --git a/tests/test_imports.py b/tests/test_imports.py new file mode 100644 index 00000000..27c701c6 --- /dev/null +++ b/tests/test_imports.py @@ -0,0 +1,43 @@ +import os +import pytest +import importlib.util + +def find_python_files(start_path): + """Recursively find all Python files in a directory.""" + py_files = [] + for root, _, files in os.walk(start_path): + for file in files: + if file.endswith(".py"): + py_files.append(os.path.join(root, file)) + return py_files + +def get_module_name_from_path(path, root_dir): + """Convert a file path to a Python module name.""" + rel_path = os.path.relpath(path, root_dir) + module_path = os.path.splitext(rel_path)[0] + return module_path.replace(os.path.sep, '.') + +SERVER_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'server')) +PYTHON_FILES = find_python_files(SERVER_DIR) + +@pytest.mark.parametrize("py_file", PYTHON_FILES) +def test_import_file(py_file): + """Test that a Python file can be imported without errors.""" + module_name = get_module_name_from_path(py_file, os.path.abspath(os.path.join(SERVER_DIR, '..'))) + + # Skip __init__.py files if they are empty or just contain comments + if os.path.basename(py_file) == '__init__.py': + with open(py_file, 'r') as f: + content = f.read().strip() + if not content or all(line.strip().startswith('#') for line in content.splitlines()): + pytest.skip(f"Skipping empty or comment-only __init__.py: {module_name}") + + try: + spec = importlib.util.spec_from_file_location(module_name, py_file) + if spec and spec.loader: + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + else: + pytest.fail(f"Could not create module spec for {py_file}") + except Exception as e: + pytest.fail(f"Failed to import {module_name} from {py_file}: {e}") From 6c9e18ad90d30e698740eba9084cc0c3e4e9e1e2 Mon Sep 17 00:00:00 2001 From: Dan Dye Date: Tue, 26 Aug 2025 23:36:11 -0400 Subject: [PATCH 4/4] Improve import validation to test tools directories - Special handling for tools/ directories: import as modules not files - This allows relative imports (from .. import utils) to work correctly - Tests 323 Python files including all tools directories - Still catches import errors like 'from typing import str' - More comprehensive coverage of the codebase --- tests/check_imports.py | 36 +++++++++++++++++++++++++++++++++-- tests/test_imports.py | 43 ------------------------------------------ 2 files changed, 34 insertions(+), 45 deletions(-) delete mode 100644 tests/test_imports.py diff --git a/tests/check_imports.py b/tests/check_imports.py index 1374748b..07bc5f75 100644 --- a/tests/check_imports.py +++ b/tests/check_imports.py @@ -21,6 +21,38 @@ def test_python_file(filepath): Returns: bool: True if import succeeds, False otherwise """ + import importlib + + # Special handling for tools directories - import as modules not files + if "/tools/" in filepath and filepath.endswith(".py"): + try: + # Convert file path to module path + # e.g., server/gti/gti_mcp/tools/files.py -> gti_mcp.tools.files + parts = filepath.replace(".py", "").split("/") + + # Find the package name (gti_mcp, secops_mcp, etc) + module_parts = [] + found_package = False + for part in parts: + if part in ["gti_mcp", "secops_mcp", "secops_soar_mcp", "scc_mcp"]: + found_package = True + if found_package: + module_parts.append(part) + + if module_parts: + # Add parent to sys.path temporarily + server_dir = filepath.split("/" + module_parts[0])[0] + if server_dir not in sys.path: + sys.path.insert(0, server_dir) + + module_name = ".".join(module_parts) + importlib.import_module(module_name) + return True + except Exception as e: + print(f"✗ {filepath}: {e}") + return False + + # Default behavior for non-tools files try: spec = importlib.util.spec_from_file_location("temp_module", filepath) if spec and spec.loader: @@ -48,8 +80,8 @@ def find_python_files(directory): python_files = [] for root, dirs, files in os.walk(directory): - # Skip test directories and tools subdirectories with relative imports - if "tests" in root or "__pycache__" in root or "/tools" in root: + # Skip test directories + if "tests" in root or "__pycache__" in root: continue for file in files: diff --git a/tests/test_imports.py b/tests/test_imports.py deleted file mode 100644 index 27c701c6..00000000 --- a/tests/test_imports.py +++ /dev/null @@ -1,43 +0,0 @@ -import os -import pytest -import importlib.util - -def find_python_files(start_path): - """Recursively find all Python files in a directory.""" - py_files = [] - for root, _, files in os.walk(start_path): - for file in files: - if file.endswith(".py"): - py_files.append(os.path.join(root, file)) - return py_files - -def get_module_name_from_path(path, root_dir): - """Convert a file path to a Python module name.""" - rel_path = os.path.relpath(path, root_dir) - module_path = os.path.splitext(rel_path)[0] - return module_path.replace(os.path.sep, '.') - -SERVER_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', 'server')) -PYTHON_FILES = find_python_files(SERVER_DIR) - -@pytest.mark.parametrize("py_file", PYTHON_FILES) -def test_import_file(py_file): - """Test that a Python file can be imported without errors.""" - module_name = get_module_name_from_path(py_file, os.path.abspath(os.path.join(SERVER_DIR, '..'))) - - # Skip __init__.py files if they are empty or just contain comments - if os.path.basename(py_file) == '__init__.py': - with open(py_file, 'r') as f: - content = f.read().strip() - if not content or all(line.strip().startswith('#') for line in content.splitlines()): - pytest.skip(f"Skipping empty or comment-only __init__.py: {module_name}") - - try: - spec = importlib.util.spec_from_file_location(module_name, py_file) - if spec and spec.loader: - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - else: - pytest.fail(f"Could not create module spec for {py_file}") - except Exception as e: - pytest.fail(f"Failed to import {module_name} from {py_file}: {e}")