test_code_quality.py 9.5 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275
  1. """
  2. Code quality tests for BamBuddy backend.
  3. These tests check for common anti-patterns and code quality issues
  4. that could cause runtime errors but aren't caught by normal tests.
  5. """
  6. import ast
  7. import os
  8. from pathlib import Path
  9. import pytest
  10. # Get the backend source directory
  11. BACKEND_DIR = Path(__file__).parent.parent.parent / "app"
  12. # Safe imports that are commonly re-imported in functions without issues
  13. # These are typically imported at the START of a function, not midway through
  14. SAFE_REIMPORT_NAMES = {
  15. "logging",
  16. "re",
  17. "os",
  18. "sys",
  19. "json",
  20. "Path",
  21. "datetime",
  22. "timedelta",
  23. "asyncio",
  24. "time",
  25. "typing",
  26. "Optional",
  27. "List",
  28. "Dict",
  29. "Any",
  30. "Union",
  31. }
  32. class DangerousImportVisitor(ast.NodeVisitor):
  33. """AST visitor that detects dangerous import patterns.
  34. Specifically looks for cases where:
  35. 1. A name is imported at module level
  36. 2. The same name is imported locally in a function
  37. 3. The name is USED before the local import in that function
  38. This pattern causes 'cannot access local variable' errors.
  39. """
  40. def __init__(self):
  41. self.module_imports: set[str] = set()
  42. self.dangerous_imports: list[tuple[str, int, str, int]] = [] # (name, import_line, function, first_use_line)
  43. self.current_function: str | None = None
  44. self.function_start_line: int = 0
  45. self.in_function = False
  46. def visit_Import(self, node: ast.Import):
  47. for alias in node.names:
  48. name = alias.asname or alias.name
  49. if not self.in_function:
  50. self.module_imports.add(name)
  51. self.generic_visit(node)
  52. def visit_ImportFrom(self, node: ast.ImportFrom):
  53. for alias in node.names:
  54. name = alias.asname or alias.name
  55. if not self.in_function:
  56. self.module_imports.add(name)
  57. self.generic_visit(node)
  58. def _check_function(self, node):
  59. """Check a function for dangerous import patterns."""
  60. if not self.in_function:
  61. return
  62. # Skip safe reimports
  63. # Collect all local imports in this function
  64. local_imports: dict[str, int] = {} # name -> line number
  65. name_uses: dict[str, int] = {} # name -> first use line number
  66. for child in ast.walk(node):
  67. # Find local imports
  68. if isinstance(child, (ast.Import, ast.ImportFrom)):
  69. for alias in child.names:
  70. name = alias.asname or alias.name
  71. if name in self.module_imports and name not in SAFE_REIMPORT_NAMES:
  72. local_imports[name] = child.lineno
  73. # Find name uses
  74. if isinstance(child, ast.Name):
  75. if child.id not in name_uses:
  76. name_uses[child.id] = child.lineno
  77. # Check for dangerous pattern: use before import
  78. for name, import_line in local_imports.items():
  79. if name in name_uses:
  80. first_use = name_uses[name]
  81. if first_use < import_line:
  82. self.dangerous_imports.append((name, import_line, self.current_function, first_use))
  83. def visit_FunctionDef(self, node: ast.FunctionDef):
  84. old_function = self.current_function
  85. old_in_function = self.in_function
  86. old_start_line = self.function_start_line
  87. self.current_function = node.name
  88. self.in_function = True
  89. self.function_start_line = node.lineno
  90. self._check_function(node)
  91. self.generic_visit(node)
  92. self.current_function = old_function
  93. self.in_function = old_in_function
  94. self.function_start_line = old_start_line
  95. def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef):
  96. old_function = self.current_function
  97. old_in_function = self.in_function
  98. old_start_line = self.function_start_line
  99. self.current_function = node.name
  100. self.in_function = True
  101. self.function_start_line = node.lineno
  102. self._check_function(node)
  103. self.generic_visit(node)
  104. self.current_function = old_function
  105. self.in_function = old_in_function
  106. self.function_start_line = old_start_line
  107. def find_import_shadowing(file_path: Path) -> list[tuple[str, int, str]]:
  108. """Find cases where local imports shadow module-level imports AND are used before import.
  109. Returns list of (name, line_number, function_name) tuples.
  110. """
  111. try:
  112. with open(file_path) as f:
  113. source = f.read()
  114. tree = ast.parse(source)
  115. visitor = DangerousImportVisitor()
  116. visitor.visit(tree)
  117. # Convert (name, import_line, function, first_use_line) to (name, import_line, function)
  118. return [(name, import_line, func) for name, import_line, func, _ in visitor.dangerous_imports]
  119. except SyntaxError:
  120. return [] # Skip files with syntax errors
  121. def get_python_files(directory: Path) -> list[Path]:
  122. """Get all Python files in a directory recursively."""
  123. return list(directory.rglob("*.py"))
  124. class TestImportShadowing:
  125. """Tests for import shadowing anti-pattern."""
  126. def test_no_import_shadowing_in_main(self):
  127. """Check main.py has no import shadowing issues.
  128. This test would have caught the ArchiveService scoping bug.
  129. """
  130. main_file = BACKEND_DIR / "main.py"
  131. if not main_file.exists():
  132. pytest.skip("main.py not found")
  133. shadows = find_import_shadowing(main_file)
  134. if shadows:
  135. error_msg = "Import shadowing detected in main.py:\n"
  136. for name, line, func in shadows:
  137. error_msg += f" - '{name}' at line {line} in function '{func}' shadows module-level import\n"
  138. error_msg += "\nThis can cause 'cannot access local variable' errors."
  139. pytest.fail(error_msg)
  140. def test_no_import_shadowing_in_services(self):
  141. """Check service files have no import shadowing issues."""
  142. services_dir = BACKEND_DIR / "services"
  143. if not services_dir.exists():
  144. pytest.skip("services directory not found")
  145. all_shadows = []
  146. for py_file in get_python_files(services_dir):
  147. shadows = find_import_shadowing(py_file)
  148. for name, line, func in shadows:
  149. all_shadows.append((py_file.name, name, line, func))
  150. if all_shadows:
  151. error_msg = "Import shadowing detected in services:\n"
  152. for filename, name, line, func in all_shadows:
  153. error_msg += f" - {filename}: '{name}' at line {line} in function '{func}'\n"
  154. pytest.fail(error_msg)
  155. def test_no_import_shadowing_in_routes(self):
  156. """Check route files have no import shadowing issues."""
  157. routes_dir = BACKEND_DIR / "api" / "routes"
  158. if not routes_dir.exists():
  159. pytest.skip("routes directory not found")
  160. all_shadows = []
  161. for py_file in get_python_files(routes_dir):
  162. shadows = find_import_shadowing(py_file)
  163. for name, line, func in shadows:
  164. all_shadows.append((py_file.name, name, line, func))
  165. if all_shadows:
  166. error_msg = "Import shadowing detected in routes:\n"
  167. for filename, name, line, func in all_shadows:
  168. error_msg += f" - {filename}: '{name}' at line {line} in function '{func}'\n"
  169. pytest.fail(error_msg)
  170. class TestModuleImports:
  171. """Tests for module import health."""
  172. def test_all_modules_importable(self):
  173. """Verify all Python modules can be imported without errors.
  174. This catches syntax errors and missing dependencies.
  175. """
  176. import importlib
  177. import sys
  178. # Modules to test importing
  179. modules = [
  180. "backend.app.main",
  181. "backend.app.services.bambu_mqtt",
  182. "backend.app.services.printer_manager",
  183. "backend.app.services.archive",
  184. "backend.app.services.notification_service",
  185. "backend.app.services.smart_plug_manager",
  186. ]
  187. errors = []
  188. for module_name in modules:
  189. try:
  190. # Remove from cache first to ensure fresh import
  191. if module_name in sys.modules:
  192. del sys.modules[module_name]
  193. importlib.import_module(module_name)
  194. except Exception as e:
  195. errors.append(f"{module_name}: {type(e).__name__}: {e}")
  196. if errors:
  197. pytest.fail("Failed to import modules:\n" + "\n".join(errors))
  198. class TestLogErrorPatterns:
  199. """Tests that use log capture to detect runtime errors."""
  200. def test_mqtt_message_processing_no_errors(self, capture_logs):
  201. """Test that MQTT message processing doesn't log errors."""
  202. from backend.app.services.bambu_mqtt import BambuMQTTClient
  203. client = BambuMQTTClient(
  204. ip_address="192.168.1.100",
  205. serial_number="TEST123",
  206. access_code="12345678",
  207. )
  208. client.on_print_start = lambda data: None
  209. client.on_print_complete = lambda data: None
  210. # Process a realistic print lifecycle
  211. messages = [
  212. {"print": {"gcode_state": "RUNNING", "gcode_file": "/test.gcode", "subtask_name": "Test"}},
  213. {"print": {"gcode_state": "RUNNING", "gcode_file": "/test.gcode", "mc_percent": 50}},
  214. {"print": {"gcode_state": "FINISH", "gcode_file": "/test.gcode", "subtask_name": "Test"}},
  215. ]
  216. for msg in messages:
  217. client._process_message(msg)
  218. assert not capture_logs.has_errors(), f"Errors during MQTT processing:\n{capture_logs.format_errors()}"