ci: Fix coverage reporting (again) (#38849)

* chore: remove _decorate_all_methods_and_functions_with_type_checker

No one understands this runtime magic anymore.

* build: Bump coverage.py to latest

* test: Skip github in coverage reporting

* test: Print traceback from all threads when test is stuck

* ci: Enable coverage in server side tests

* ci: Always enable coverage

It's cheap in recent python versions, our reasons for selectively
disabling aren't valid anymore.

* ci: Disable stderr capturing

* ci: Use default buffer behaviour in unittest runner

* ci(coverage): Set concurrency to multiprocessing

We do use multiprocessing, perhaps the patches aren't concurrectly
handled?

* ci(coverage): Try parallel run

* fix: Apply subprocess patch

* ci: Don't start web server with coverage

Causes deadlock for some reason. We don't actually report it either.

* ci: only submit UI coverage if ran

* test: remove aggresive stuck test checking

* ci: disable UI coverage

(for now)

.github/workflows/_base-server-tests.yml

@@ -29,7 +29,7 @@ on:
       enable-coverage:
         required: false
         type: boolean
-        default: false
+        default: true
 
 
 jobs:
@@ -100,7 +100,6 @@ jobs:
           python-version: ${{ inputs.python-version }}
           node-version: ${{ inputs.node-version }}
           disable-socketio: true
-          enable-coverage: ${{ inputs.enable-coverage }}
           db-root-password: ${{ env.DB_ROOT_PASSWORD }}
           db: ${{ matrix.db }}
         env:
@@ -110,29 +109,11 @@ jobs:
         run: |
           bench --site test_site \
             run-parallel-tests \
+            --with-coverage \
             --app "${{ github.event.repository.name }}" \
             --total-builds ${{ inputs.parallel-runs }} \
-            --build-number ${{ matrix.index }} 2> >(tee -a stderr.log >&2)
+            --build-number ${{ matrix.index }}
 
-          # Process warnings and create annotations
-          if [ -s stderr.log ] && [ "$DB" == "mariadb" ]; then
-            echo "Processing deprecation warnings..."
-            grep -E "DeprecationWarning" stderr.log | sort -u | while read -r warning; do
-              # Extract file path, line number, and warning type
-              file_info=$(echo "$warning" | grep -oP '^.*?:\d+:')
-              file_path=$(echo "$file_info" | cut -d':' -f1)
-              line_number=$(echo "$file_info" | cut -d':' -f2)
-              warning_type=$(echo "$warning" | grep -oP '\w+Warning')
-
-              # Extract the actual warning message
-              message=$(echo "$warning" | sed -E "s/^.*$warning_type: //")
-
-              # Create the annotation
-              echo "::warning file=${file_path},line=${line_number}::${warning_type}: ${message}"
-            done
-          else
-            echo "No deprecation warnings found."
-          fi
         env:
           DB: ${{ matrix.db }}
           # consumed by bench run-parallel-tests
@@ -144,7 +125,7 @@ jobs:
         if: inputs.enable-coverage
         with:
           name: coverage-${{ matrix.db }}-${{ matrix.index }}
-          path: ./sites/*-coverage*.xml
+          path: ./sites/*coverage*.xml
 
       - name: Setup tmate session
         uses: mxschmitt/action-tmate@v3

.github/workflows/server-tests.yml

@@ -48,7 +48,6 @@ jobs:
       enable-postgres: ${{ needs.checkrun.outputs.run_postgres == 'true' }} # This enables PostgreSQL to run tests
       enable-sqlite: false  # This will test against both MariaDB and SQLite if enabled
       parallel-runs: 2
-      enable-coverage: ${{ github.event_name != 'pull_request' }}
       fake-success: ${{ needs.checkrun.outputs.build != 'strawberry' }}
     needs: checkrun
     secrets: inherit
@@ -67,7 +66,6 @@ jobs:
     name: Coverage Wrap Up
     needs: [test, checkrun]
     runs-on: ubuntu-latest
-    if: ${{ github.event_name != 'pull_request' }}
     steps:
       - name: Clone
         uses: actions/checkout@v6

.github/workflows/ui-tests.yml

@@ -44,14 +44,14 @@ jobs:
     uses: ./.github/workflows/_base-ui-tests.yml
     with:
       parallel-runs: 3
-      enable-coverage: ${{ github.event_name != 'pull_request' }}
+      enable-coverage: false
       fake-success: ${{ needs.checkrun.outputs.build != 'strawberry' }}
     needs: checkrun
 
   coverage:
     name: Coverage Wrap Up
     needs: [test, checkrun]
-    if: ${{ github.event_name != 'pull_request' }}
+    if: ${{ needs.checkrun.outputs.build == 'strawberry' }}
     runs-on: ubuntu-latest
     steps:
       - name: Clone

codecov.yml

@@ -29,10 +29,6 @@ flags:
     paths:
       - "**/*.py"
     carryforward: true
-  ui-tests:
-    paths:
-      - "**/*.js"
-    carryforward: true
   server-ui:
     paths:
       - "**/*.py"

frappe/commands/testing.py

@@ -144,7 +144,6 @@ def main(
 			verbosity=2 if testing_module_logger.getEffectiveLevel() < logging.INFO else 1,
 			tb_locals=testing_module_logger.getEffectiveLevel() <= logging.INFO,
 			cfg=test_config,
-			buffer=not debug,  # unfortunate as it messes up stdout/stderr output order
 		)
 
 		if doctype or doctype_list_path:

frappe/coverage.py

@@ -22,6 +22,7 @@ STANDARD_EXCLUSIONS = [
 	"*/node_modules/*",
 	"*/doctype/*/*_dashboard.py",
 	"*/patches/*",
+	"*/.github/*",
 ]
 
 # tested via commands' test suite
@@ -78,7 +79,12 @@ class CodeCoverage:
 			if self.app == "frappe":
 				omit.extend(FRAPPE_EXCLUSIONS)
 
-			self.coverage = Coverage(source=[source_path], omit=omit, include=STANDARD_INCLUSIONS)
+			self.coverage = Coverage(
+				source=[source_path],
+				omit=omit,
+				include=STANDARD_INCLUSIONS,
+				data_suffix=True,
+			)
 			self.coverage.start()
 		return self
 

frappe/parallel_test_runner.py

@@ -14,7 +14,6 @@ import requests
 import frappe
 from frappe.tests.utils import make_test_records, toggle_test_mode
 
-from .testing.environment import _decorate_all_methods_and_functions_with_type_checker
 from .testing.result import TestResult
 
 click_ctx = click.get_current_context(True)
@@ -61,7 +60,6 @@ class ParallelTestRunner:
 		frappe.clear_cache()
 		frappe.utils.scheduler.disable_scheduler()
 		if not self.lightmode:
-			_decorate_all_methods_and_functions_with_type_checker()
 			self.before_test_setup()
 
 	def before_test_setup(self):

frappe/testing/environment.py

@@ -56,8 +56,6 @@ def _initialize_test_environment(site, config):
 	frappe.flags.print_messages = logger.getEffectiveLevel() < logging.INFO
 	frappe.flags.tests_verbose = logger.getEffectiveLevel() < logging.INFO
 
-	_decorate_all_methods_and_functions_with_type_checker()
-
 
 def _cleanup_after_tests():
 	"""Perform cleanup operations after running tests"""
@@ -83,95 +81,6 @@ def _disable_scheduler_if_needed():
 		frappe.utils.scheduler.disable_scheduler()
 
 
-@debug_timer
-def _decorate_all_methods_and_functions_with_type_checker():
-	from frappe.utils.typing_validations import validate_argument_types
-
-	def _get_config_from_pyproject(app_path):
-		try:
-			with open(f"{app_path}/pyproject.toml", "rb") as f:
-				config = tomllib.load(f)
-				return (
-					config.get("tool", {})
-					.get("frappe", {})
-					.get("testing", {})
-					.get("function_type_validation", {})
-				)
-		except FileNotFoundError:
-			return {}
-		except tomllib.TOMLDecodeError:
-			logger.warning(f"Failed to parse pyproject.toml for app {app_path}")
-			return {}
-
-	def _decorate_callable(obj, parent_module):
-		# whitelisted methods are already checked, see frappe.whitelist
-		if getattr(obj, "__func__", obj) in frappe.whitelisted:
-			return obj
-		# Check if the function is already decorated
-		elif hasattr(obj, "_is_decorated_for_validate_argument_types"):
-			return obj
-		elif module := getattr(obj, "__module__", ""):
-			# ensure that the origin skip list is honored on imports; but not the origin
-			# max_depth because they are reimported thus attached to a different namespace
-			app = module.split(".", 1)[0]
-			config = _get_config_from_pyproject(frappe.get_app_source_path(app))
-			skip_namespaces = config.get("skip_namespaces", [])
-			if any(module.startswith(n) for n in skip_namespaces):
-				return obj
-
-		@functools.wraps(obj)
-		def wrapper(*args, **kwargs):
-			return validate_argument_types(obj)(*args, **kwargs)
-
-		wrapper._is_decorated_for_validate_argument_types = True
-
-		if obj.__module__ != parent_module.__name__:
-			logger.debug(f"... patching {obj.__module__}.{obj.__name__} (inside {parent_module.__name__})")
-		else:
-			logger.debug(f"... patching {obj.__module__}.{obj.__name__}")
-
-		return wrapper
-
-	def _decorate_module(app, module, apps, current_depth, max_depth):
-		if current_depth > max_depth:
-			return
-		for name in dir(module):
-			obj = getattr(module, name)
-			if inspect.isfunction(obj):
-				if not getattr(obj, "__annotations__", None):
-					continue
-				# never cross the apps (plural!) boundary for functions
-				if obj.__module__.split(".", 1)[0] not in apps:
-					continue
-				setattr(module, name, _decorate_callable(obj, module))
-			elif inspect.ismodule(obj):
-				# never cross the app (singular!) boundary for modules
-				if obj.__name__.split(".", 1)[0] != app:
-					continue
-				if hasattr(obj, "_is_decorated_for_validate_argument_types"):
-					continue
-				obj._is_decorated_for_validate_argument_types = True
-				_decorate_module(app, obj, apps, current_depth + 1, max_depth)
-
-	for app in (apps := frappe.get_installed_apps()):
-		config = _get_config_from_pyproject(frappe.get_app_source_path(app))
-		max_depth = config.get("max_module_depth", 0)
-		skip_namespaces = config.get("skip_namespaces", [])
-		logger.info(f"Adding type validator in {app!r} (up to level {max_depth})...")
-		pkg = frappe.get_module(app)
-		_decorate_module(app, pkg, apps, 1, max_depth)
-
-		for _, submodule_name, _ in pkgutil.walk_packages(path=pkg.__path__, prefix=pkg.__name__ + "."):
-			current_depth = len(submodule_name.split("."))
-			if current_depth > max_depth:
-				continue
-			if any(submodule_name.startswith(n) for n in skip_namespaces):
-				continue
-
-			submodule = frappe.get_module(submodule_name)
-			_decorate_module(app, submodule, apps, current_depth, max_depth)
-
-
 class IntegrationTestPreparation:
 	def __init__(self, cfg):
 		self.cfg = cfg

frappe/testing/runner.py

@@ -52,7 +52,7 @@ class TestRunner(unittest.TextTestRunner):
 		descriptions=True,
 		verbosity=1,
 		failfast=False,
-		buffer=True,
+		buffer=False,
 		resultclass=None,
 		warnings="module",
 		*,

frappe/tests/classes/context_managers.py

@@ -1,3 +1,4 @@
+import faulthandler
 import logging
 from collections.abc import Callable
 from contextlib import contextmanager

pyproject.toml

@@ -129,7 +129,7 @@ dev = [
 ]
 test = [
     "unittest-xml-reporting~=3.2.0",
-    "coverage~=7.10.0",
+    "coverage~=7.13.5",
     "hypothesis~=6.77.0",
     "freezegun~=1.5.1",
 ]
@@ -146,7 +146,7 @@ skip_namespaces = [
 ]
 
 [tool.bench.dev-dependencies]
-coverage = "~=7.10.0"
+coverage = "~=7.13.5"
 pyngrok = "~=7.5.0"
 unittest-xml-reporting = "~=3.2.0"
 watchdog = "~=6.0.0"