test_code_quality.py 9.5 KB

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