From 88d83c10a4ecfb2df52a9d383db41aded9f5aa0d Mon Sep 17 00:00:00 2001 From: Dominic R Date: Wed, 11 Jun 2025 13:14:38 -0400 Subject: [PATCH] root: test label handling and error reporting in PytestTestRunner (#14000) * root: test label handling and error reporting in PytestTestRunner - Added a check for empty test labels and return a clear error instead of failing silently. - Improved handling of dotted module paths + included support for single-module names. - Replaced a RuntimeError with a printed error message when a test label can't be resolved. - Wrapped the pytest.main call in a try/except to catch unexpected errors and print them nicely. * Fix handling for test file without extension and lint * oops * little improvments too --- authentik/root/test_runner.py | 98 ++++++++++++++++++++++++----------- 1 file changed, 69 insertions(+), 29 deletions(-) diff --git a/authentik/root/test_runner.py b/authentik/root/test_runner.py index dd749dfbaa..a46e207f8e 100644 --- a/authentik/root/test_runner.py +++ b/authentik/root/test_runner.py @@ -7,6 +7,7 @@ from unittest import TestCase import pytest from django.conf import settings from django.test.runner import DiscoverRunner +from structlog.stdlib import get_logger from authentik.lib.config import CONFIG from authentik.lib.sentry import sentry_init @@ -22,6 +23,7 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover def __init__(self, **kwargs): super().__init__(**kwargs) + self.logger = get_logger().bind(runner="pytest") self.args = [] if self.failfast: @@ -34,22 +36,33 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover if kwargs.get("no_capture", False): self.args.append("--capture=no") + self._setup_test_environment() + + def _setup_test_environment(self): + """Configure test environment settings""" settings.TEST = True settings.CELERY["task_always_eager"] = True - CONFIG.set("events.context_processors.geoip", "tests/GeoLite2-City-Test.mmdb") - CONFIG.set("events.context_processors.asn", "tests/GeoLite2-ASN-Test.mmdb") - CONFIG.set("blueprints_dir", "./blueprints") - CONFIG.set( - "outposts.container_image_base", - f"ghcr.io/goauthentik/dev-%(type)s:{get_docker_tag()}", - ) - CONFIG.set("tenants.enabled", False) - CONFIG.set("outposts.disable_embedded_outpost", False) - CONFIG.set("error_reporting.sample_rate", 0) - CONFIG.set("error_reporting.environment", "testing") - CONFIG.set("error_reporting.send_pii", True) - sentry_init() + # Test-specific configuration + test_config = { + "events.context_processors.geoip": "tests/GeoLite2-City-Test.mmdb", + "events.context_processors.asn": "tests/GeoLite2-ASN-Test.mmdb", + "blueprints_dir": "./blueprints", + "outposts.container_image_base": f"ghcr.io/goauthentik/dev-%(type)s:{get_docker_tag()}", + "tenants.enabled": False, + "outposts.disable_embedded_outpost": False, + "error_reporting.sample_rate": 0, + "error_reporting.environment": "testing", + "error_reporting.send_pii": True, + } + + for key, value in test_config.items(): + CONFIG.set(key, value) + + sentry_init() + self.logger.debug("Test environment configured") + + # Send startup signals pre_startup.send(sender=self, mode="test") startup.send(sender=self, mode="test") post_startup.send(sender=self, mode="test") @@ -72,7 +85,21 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover help="Disable any capturing of stdout/stderr during tests.", ) - def run_tests(self, test_labels, extra_tests=None, **kwargs): + def _validate_test_label(self, label: str) -> bool: + """Validate test label format""" + if not label: + return False + + # Check for invalid characters, but allow forward slashes and colons + # for paths and pytest markers + invalid_chars = set('\\*?"<>|') + if any(c in label for c in invalid_chars): + self.logger.error("Invalid characters in test label", label=label) + return False + + return True + + def run_tests(self, test_labels: list[str], extra_tests=None, **kwargs): """Run pytest and return the exitcode. It translates some of Django's test command option to pytest's. @@ -82,10 +109,17 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover The extra_tests argument has been deprecated since Django 5.x It is kept for compatibility with PyCharm's Django test runner. """ + if not test_labels: + self.logger.error("No test files specified") + return 1 for label in test_labels: + if not self._validate_test_label(label): + return 1 + valid_label_found = False label_as_path = os.path.abspath(label) + # File path has been specified if os.path.exists(label_as_path): self.args.append(label_as_path) @@ -93,24 +127,30 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover elif "::" in label: self.args.append(label) valid_label_found = True - # Convert dotted module path to file_path::class::method else: + # Check if the label is a dotted module path path_pieces = label.split(".") - # Check whether only class or class and method are specified for i in range(-1, -3, -1): - path = os.path.join(*path_pieces[:i]) + ".py" - label_as_path = os.path.abspath(path) - if os.path.exists(label_as_path): - path_method = label_as_path + "::" + "::".join(path_pieces[i:]) - self.args.append(path_method) - valid_label_found = True - break + try: + path = os.path.join(*path_pieces[:i]) + ".py" + if os.path.exists(path): + if i < -1: + path_method = path + "::" + "::".join(path_pieces[i:]) + self.args.append(path_method) + else: + self.args.append(path) + valid_label_found = True + break + except (TypeError, IndexError): + continue if not valid_label_found: - raise RuntimeError( - f"One of the test labels: {label!r}, " - f"is not supported. Use a dotted module name or " - f"path instead." - ) + self.logger.error("Test file not found", label=label) + return 1 - return pytest.main(self.args) + self.logger.info("Running tests", test_files=self.args) + try: + return pytest.main(self.args) + except Exception as e: + self.logger.error("Error running tests", error=str(e), test_files=self.args) + return 1