diff --git a/appmap/_implementation/__init__.py b/appmap/_implementation/__init__.py index 9911d333..da358a99 100644 --- a/appmap/_implementation/__init__.py +++ b/appmap/_implementation/__init__.py @@ -1,6 +1,7 @@ from . import configuration from . import env as appmapenv from . import event, importer, metadata, recorder +from .detect_enabled import DetectEnabled from .py_version_check import check_py_version @@ -12,6 +13,7 @@ def initialize(**kwargs): recorder.initialize() configuration.initialize() # needs to be initialized after recorder metadata.initialize() + DetectEnabled.initialize() initialize() diff --git a/appmap/_implementation/detect_enabled.py b/appmap/_implementation/detect_enabled.py new file mode 100644 index 00000000..229ea980 --- /dev/null +++ b/appmap/_implementation/detect_enabled.py @@ -0,0 +1,183 @@ +""" +Detect if AppMap is enabled. +""" + +import importlib +import logging +import os +from textwrap import dedent + +logger = logging.getLogger(__name__) + +RECORDING_METHODS = ["pytest", "unittest", "remote", "requests"] + +# Detects whether AppMap recording should be enabled. This test can be +# performed generally, or for a particular recording method. Recording +# can be enabled explicitly, for example via APPMAP=true, or it can be +# enabled implicitly, by running in a dev or test web application +# environment. Recording can also be disabled explicitly, using +# environment variables. +class DetectEnabled: + _instance = None + + def __new__(cls): + if cls._instance is None: + logger.debug("Creating the DetectEnabled object") + cls._instance = super(DetectEnabled, cls).__new__(cls) + cls._instance._initialized = False + return cls._instance + + def __init__(self): + if self._initialized: + return + + self._initialized = True + self._detected_for_method = {} + + @classmethod + def initialize(cls): + cls._instance = None + # because apparently __new__ and __init__ don't get called + cls._detected_for_method = {} + + @classmethod + def clear_cache(cls): + cls._detected_for_method = {} + + @classmethod + def is_appmap_repo(cls): + return os.path.exists("appmap/__init__.py") and os.path.exists( + "appmap/_implementation/__init__.py" + ) + + @classmethod + def should_enable(cls, recording_method): + """ + Should recording be enabled for the current recording method? + """ + if recording_method in cls._detected_for_method: + return cls._detected_for_method[recording_method] + else: + message, enabled = cls.detect_should_enable(recording_method) + cls._detected_for_method[recording_method] = enabled + if enabled: + logger.warning(dedent(f"AppMap recording is enabled because {message}")) + return enabled + + @classmethod + def detect_should_enable(cls, recording_method): + if not recording_method: + return ["no recording method is set", False] + + if recording_method not in RECORDING_METHODS: + return ["invalid recording method", False] + + # explicitly disabled or enabled + if "APPMAP" in os.environ: + if os.environ["APPMAP"] == "false": + return ["APPMAP=false", False] + elif os.environ["APPMAP"] == "true": + return ["APPMAP=true", True] + + # recording method explicitly disabled or enabled + if recording_method: + for one_recording_method in RECORDING_METHODS: + if one_recording_method == recording_method.lower(): + env_var = "_".join(["APPMAP", "RECORD", recording_method.upper()]) + if env_var in os.environ: + if os.environ[env_var] == "false": + return [f"{env_var}=false", False] + elif os.environ[env_var] == "true": + return [f"{env_var}=true", True] + + # it's flask + message, should_enable = cls.is_flask_and_should_enable() + if should_enable == True or should_enable == False: + return [message, should_enable] + + # it's django + message, should_enable = cls.is_django_and_should_enable() + if should_enable == True or should_enable == False: + return [message, should_enable] + + if recording_method in RECORDING_METHODS: + return ["will record by default", True] + + return ["it's not enabled by any configuration or framework", False] + + @classmethod + def is_flask_and_should_enable(cls): + if "FLASK_DEBUG" in os.environ: + if os.environ["FLASK_DEBUG"] == "1": + return [f"FLASK_DEBUG={os.environ['FLASK_DEBUG']}", True] + elif os.environ["FLASK_DEBUG"] == "0": + return [f"FLASK_DEBUG={os.environ['FLASK_DEBUG']}", False] + + if "FLASK_ENV" in os.environ: + if os.environ["FLASK_ENV"] == "development": + return [f"FLASK_ENV={os.environ['FLASK_ENV']}", True] + elif os.environ["FLASK_ENV"] == "production": + return [f"FLASK_ENV={os.environ['FLASK_ENV']}", False] + + return ["it's not Flask", None] + + @classmethod + def is_django_and_should_enable(cls): + if ( + "DJANGO_SETTINGS_MODULE" in os.environ + and os.environ["DJANGO_SETTINGS_MODULE"] != "" + ): + try: + settings = importlib.import_module(os.environ["DJANGO_SETTINGS_MODULE"]) + except Exception as exn: + settings = None + return [ + "couldn't load DJANGO_SETTINGS_MODULE={os.environ['DJANGO_SETTINGS_MODULE']}", + False, + ] + + if settings: + try: + # don't crash if the settings file doesn't contain + # a DEBUG variable + if settings.DEBUG == True: + return [ + f"{os.environ['DJANGO_SETTINGS_MODULE']}.DEBUG={settings.DEBUG}", + True, + ] + elif settings.DEBUG == False: + return [ + f"{os.environ['DJANGO_SETTINGS_MODULE']}.DEBUG={settings.DEBUG}", + False, + ] + except AttributeError as exn: + # it wasn't set. it's ok. don't crash + # AttributeError: module 'app.settings_appmap_false' has no attribute 'DEBUG' + pass + + if settings: + try: + # don't crash if the settings file doesn't contain + # an APPMAP variable + if ( + settings.APPMAP == True + or str(settings.APPMAP).upper() == "true".upper() + ): + return [ + f"{os.environ['DJANGO_SETTINGS_MODULE']}.APPMAP={settings.APPMAP}", + True, + ] + elif ( + settings.APPMAP == False + or str(settings.APPMAP).upper() == "false".upper() + ): + return [ + f"{os.environ['DJANGO_SETTINGS_MODULE']}.APPMAP={settings.APPMAP}", + False, + ] + except AttributeError as exn: + # it wasn't set. it's ok. don't crash + # AttributeError: module 'app.settings_appmap_false' has no attribute 'APPMAP' + pass + + return ["it's not Django", None] diff --git a/appmap/_implementation/testing_framework.py b/appmap/_implementation/testing_framework.py index 90cb83e5..88baee40 100644 --- a/appmap/_implementation/testing_framework.py +++ b/appmap/_implementation/testing_framework.py @@ -1,5 +1,6 @@ """Shared infrastructure for testing framework integration.""" +import os import re from contextlib import contextmanager @@ -99,10 +100,6 @@ def __init__(self, name, recorder_type, version=None): @contextmanager def record(self, klass, method, **kwds): - if not env.Env.current.enabled: - yield - return - Metadata.add_framework(self.name, self.version) item = FuncItem(klass, method, **kwds) @@ -142,3 +139,9 @@ def collect_result_metadata(metadata): metadata["test_status"] = "failed" metadata["exception"] = {"class": exn.__class__.__name__, "message": str(exn)} raise + +def file_delete(filename): + try: + os.remove(filename) + except FileNotFoundError: + pass diff --git a/appmap/_implementation/web_framework.py b/appmap/_implementation/web_framework.py index 20eed32c..36ec44c2 100644 --- a/appmap/_implementation/web_framework.py +++ b/appmap/_implementation/web_framework.py @@ -10,6 +10,7 @@ from tempfile import NamedTemporaryFile from appmap._implementation import generation +from appmap._implementation.detect_enabled import DetectEnabled from appmap._implementation.env import Env from appmap._implementation.event import Event, ReturnEvent, _EventIds, describe_value from appmap._implementation.recorder import Recorder, ThreadRecorder @@ -130,32 +131,36 @@ def create_appmap_file( class AppmapMiddleware(ABC): - def before_request_hook( - self, request, request_path, record_url, recording_is_running - ): - if request_path == record_url: + def __init__(self): + self.record_url = "/_appmap/record" + + def should_record(self): + return DetectEnabled.should_enable("remote") or DetectEnabled.should_enable("requests") + + def before_request_hook(self, request, request_path, recording_is_running): + if request_path == self.record_url: return None, None, None + rec = None start = None call_event_id = None - if Env.current.enabled or recording_is_running: - # It should be recording or it's currently recording. The - # recording is either - # a) remote, enabled by POST to /_appmap/record, which set - # recording_is_running, or - # b) requests, set by Env.current.record_all_requests, or - # c) both remote and requests; there are multiple active recorders. - if not Env.current.record_all_requests and recording_is_running: - # a) - rec = Recorder.get_current() - else: - # b) or c) - rec = ThreadRecorder() - Recorder.set_current(rec) - rec.start_recording() - - if rec.get_enabled(): - start, call_event_id = self.before_request_main(rec, request) + if DetectEnabled.should_enable("requests"): + # a) requests + rec = ThreadRecorder() + Recorder.set_current(rec) + rec.start_recording() + # Each time an event is added for a thread_id it's also + # added to the global Recorder(). So don't add the event + # to the global Recorder() explicitly because that would + # add the event in it twice. + elif DetectEnabled.should_enable("remote") or recording_is_running: + # b) APPMAP=true, or + # c) remote, enabled by POST to /_appmap/record, which set + # recording_is_running + rec = Recorder.get_current() + + if rec and rec.get_enabled(): + start, call_event_id = self.before_request_main(rec, request) return rec, start, call_event_id @@ -167,7 +172,6 @@ def after_request_hook( self, request, request_path, - record_url, recording_is_running, request_method, request_base_url, @@ -176,44 +180,39 @@ def after_request_hook( start, call_event_id, ): - if request_path == record_url: + if request_path == self.record_url: return response - if Env.current.enabled or recording_is_running: - # It should be recording or it's currently recording. The - # recording is either - # a) remote, enabled by POST to /_appmap/record, which set - # self.recording.is_running, or - # b) requests, set by Env.current.record_all_requests, or - # c) both remote and requests; there are multiple active recorders. - if not Env.current.record_all_requests and recording_is_running: - # a) - rec = Recorder.get_current() + if DetectEnabled.should_enable("requests"): + # a) requests + rec = Recorder.get_current() + # Each time an event is added for a thread_id it's also + # added to the global Recorder(). So don't add the event + # to the global Recorder() explicitly because that would + # add the event in it twice. + try: if rec.get_enabled(): self.after_request_main(rec, response, start, call_event_id) - else: - # b) or c) - rec = Recorder.get_current() - # Each time an event is added for a thread_id it's also - # added to the global Recorder(). So don't add the event - # to the global Recorder() explicitly because that would - # add the event in it twice. - try: - if rec.get_enabled(): - self.after_request_main(rec, response, start, call_event_id) - - output_dir = Env.current.output_dir / "requests" - create_appmap_file( - output_dir, - request_method, - request_path, - request_base_url, - response, - response_headers, - rec, - ) - finally: - rec.stop_recording() + + output_dir = Env.current.output_dir / "requests" + create_appmap_file( + output_dir, + request_method, + request_path, + request_base_url, + response, + response_headers, + rec, + ) + finally: + rec.stop_recording() + elif DetectEnabled.should_enable("remote") or recording_is_running: + # b) APPMAP=true, or + # c) remote, enabled by POST to /_appmap/record, which set + # recording_is_running + rec = Recorder.get_current() + if rec.get_enabled(): + self.after_request_main(rec, response, start, call_event_id) return response diff --git a/appmap/django.py b/appmap/django.py index 6d881fd8..4e640a65 100644 --- a/appmap/django.py +++ b/appmap/django.py @@ -22,7 +22,7 @@ from django.urls.exceptions import Resolver404 from django.urls.resolvers import _route_to_regex -from appmap._implementation import generation, recorder, web_framework +from appmap._implementation import generation, recording, web_framework from appmap._implementation.env import Env from appmap._implementation.event import ( ExceptionEvent, @@ -225,19 +225,19 @@ class Middleware(AppmapMiddleware): """ def __init__(self, get_response): + super().__init__() self.get_response = get_response self.recorder = Recorder.get_current() - self.record_url = "/_appmap/record" def __call__(self, request): - if not Env.current.enabled: + if not self.should_record(): return self.get_response(request) if request.path_info == self.record_url: return self.recording(request) rec, start, call_event_id = self.before_request_hook( - request, request.path_info, self.record_url, self.recorder.get_enabled() + request, request.path_info, self.recorder.get_enabled() ) try: @@ -256,7 +256,6 @@ def __call__(self, request): self.after_request_hook( request, request.path_info, - self.record_url, self.recorder.get_enabled(), request.method, request.build_absolute_uri(), diff --git a/appmap/flask.py b/appmap/flask.py index f32b9bb3..a8fc7349 100644 --- a/appmap/flask.py +++ b/appmap/flask.py @@ -12,10 +12,10 @@ from werkzeug.routing import parse_rule from appmap._implementation import generation, web_framework +from appmap._implementation.detect_enabled import DetectEnabled from appmap._implementation.env import Env from appmap._implementation.event import HttpServerRequestEvent, HttpServerResponseEvent from appmap._implementation.recorder import Recorder -from appmap._implementation.recording import Recording from appmap._implementation.web_framework import AppmapMiddleware from appmap._implementation.web_framework import TemplateHandler as BaseTemplateHandler @@ -47,68 +47,67 @@ def request_params(req): class AppmapFlask(AppmapMiddleware): def __init__(self, app=None): + super().__init__() self.app = app if app is not None: self.init_app(app) def init_app(self, app): - if not Env.current.enabled: - return - - if not app: - return - - self.recording = Recording() - - self.record_url = "/_appmap/record" - - # print('in init_app') - app.add_url_rule( - self.record_url, - "appmap_record_get", - view_func=self.record_get, - methods=["GET"], - ) - app.add_url_rule( - self.record_url, - "appmap_record_post", - view_func=self.record_post, - methods=["POST"], - ) - app.add_url_rule( - self.record_url, - "appmap_record_delete", - view_func=self.record_delete, - methods=["DELETE"], - ) + if self.should_record(): + # it may record requests but not remote (APPMAP=false) + self.recorder = Recorder.get_current() + + if Env.current.enabled: + # the remote recording routes are enabled only if APPMAP=true + app.add_url_rule( + self.record_url, + "appmap_record_get", + view_func=self.record_get, + methods=["GET"], + ) + app.add_url_rule( + self.record_url, + "appmap_record_post", + view_func=self.record_post, + methods=["POST"], + ) + app.add_url_rule( + self.record_url, + "appmap_record_delete", + view_func=self.record_delete, + methods=["DELETE"], + ) app.before_request(self.before_request) app.after_request(self.after_request) def record_get(self): - return {"enabled": self.recording.is_running()} + if not self.should_record(): + return "Appmap is disabled.", 404 + + return {"enabled": self.recorder.get_enabled()} def record_post(self): - if self.recording.is_running(): + if self.recorder.get_enabled(): return "Recording is already in progress", 409 - self.recording.start() + self.recorder.start_recording() return "", 200 def record_delete(self): - if not self.recording.is_running(): + if not self.recorder.get_enabled(): return "No recording is in progress", 404 - self.recording.stop() + self.recorder.stop_recording() - return json.loads(generation.dump(self.recording)) + return json.loads(generation.dump(self.recorder)) def before_request(self): - if not Env.current.enabled: + if not self.should_record(): return rec, start, call_event_id = self.before_request_hook( - request, request.path, self.record_url, self.recording.is_running() + request, request.path, self.recorder.get_enabled() ) def before_request_main(self, rec, request): @@ -141,14 +140,13 @@ def before_request_main(self, rec, request): return None, None def after_request(self, response): - if not Env.current.enabled: - return + if not self.should_record(): + return response return self.after_request_hook( request, request.path, - self.record_url, - self.recording.is_running(), + self.recorder.get_enabled(), request.method, request.base_url, response, diff --git a/appmap/pytest.py b/appmap/pytest.py index 6f941db5..76b2726c 100644 --- a/appmap/pytest.py +++ b/appmap/pytest.py @@ -3,6 +3,7 @@ import appmap import appmap.wrapt as wrapt from appmap._implementation import testing_framework +from appmap._implementation.detect_enabled import DetectEnabled class recorded_testcase: @@ -19,7 +20,7 @@ def __call__(self, wrapped, _, args, kwargs): return wrapped(*args, **kwargs) -if appmap.enabled(): +if not DetectEnabled.is_appmap_repo() and DetectEnabled.should_enable("pytest"): @pytest.hookimpl def pytest_sessionstart(session): diff --git a/appmap/test/conftest.py b/appmap/test/conftest.py index d3dd02c5..0988f8b6 100644 --- a/appmap/test/conftest.py +++ b/appmap/test/conftest.py @@ -91,3 +91,12 @@ def git_directory_fixture(tmp_path_factory): def tmp_git(git_directory, tmp_path): copy_tree(git_directory, str(tmp_path)) return utils.git(cwd=tmp_path) + + +# fix the following error: +# AttributeError: module 'django.core.mail' has no attribute 'outbox' +# https://github.com/pytest-dev/pytest-django/issues/993#issue-1147362573 +@pytest.fixture(scope="function", autouse=True) +def _dj_autoclear_mailbox() -> None: + # Override the `_dj_autoclear_mailbox` test fixture in `pytest_django`. + pass diff --git a/appmap/test/test_detect_enabled.py b/appmap/test/test_detect_enabled.py new file mode 100644 index 00000000..3cc6e321 --- /dev/null +++ b/appmap/test/test_detect_enabled.py @@ -0,0 +1,179 @@ +""" +Test detecting if AppMap is enabled +""" +import os +import unittest +from unittest.mock import patch + +import pytest + +from appmap._implementation.detect_enabled import RECORDING_METHODS, DetectEnabled +from appmap._implementation.testing_framework import file_delete + + +class TestDetectEnabled: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + @patch.dict(os.environ, {"APPMAP": "false"}) + def test_none__appmap_disabled(self): + assert DetectEnabled.should_enable(None) == False + + @patch.dict(os.environ, {"APPMAP": "true"}) + def test_none__appmap_enabled(self): + # if there's no recording method then it's disabled + assert DetectEnabled.should_enable(None) == False + + def test_invalid__no_envvar(self): + assert DetectEnabled.should_enable("invalid_recording_method") == False + + @patch.dict(os.environ, {"APPMAP": "true"}) + def test_invalid__appmap_enabled(self): + assert DetectEnabled.should_enable("invalid_recording_method") == False + + @patch.dict(os.environ, {"APPMAP": "false"}) + def test_some__appmap_disabled(self): + for recording_method in RECORDING_METHODS: + assert DetectEnabled.should_enable(recording_method) == False + + @patch.dict(os.environ, {"APPMAP": "true"}) + def test_some__appmap_enabled(self): + for recording_method in RECORDING_METHODS: + assert DetectEnabled.should_enable(recording_method) == True + + recording_methods_as_true = { + "_".join(["APPMAP", "RECORD", recording_method.upper()]): "true" + for recording_method in RECORDING_METHODS + } + + @patch.dict(os.environ, recording_methods_as_true) + def test_some__recording_method_enabled(self): + for recording_method in RECORDING_METHODS: + assert DetectEnabled.should_enable(recording_method) == True + + recording_methods_as_false = { + "_".join(["APPMAP", "RECORD", recording_method.upper()]): "false" + for recording_method in RECORDING_METHODS + } + + @patch.dict(os.environ, recording_methods_as_false) + def test_some__recording_method_disabled(self): + for recording_method in RECORDING_METHODS: + assert DetectEnabled.should_enable(recording_method) == False + + +class TestDetectEnabledFlask: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + def test__none(self): + assert DetectEnabled.should_enable(None) == False + + @patch.dict(os.environ, {"FLASK_APP": "app.py"}) + def test__flask_app(self): + assert DetectEnabled.should_enable(None) == False + + @patch.dict(os.environ, {"FLASK_DEBUG": "0"}) + def test__flask_debug_0(self): + assert DetectEnabled.should_enable("requests") == False + + @patch.dict(os.environ, {"FLASK_DEBUG": "1"}) + def test__flask_debug_1(self): + assert DetectEnabled.should_enable("requests") == True + + @patch.dict(os.environ, {"FLASK_ENV": "production"}) + def test__flask_env_production(self): + assert DetectEnabled.should_enable("requests") == False + + @patch.dict(os.environ, {"FLASK_ENV": "development"}) + def test__flask_env_development(self): + assert DetectEnabled.should_enable("requests") == True + + +class TestDetectEnabledDjango: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + def test__none(self): + assert DetectEnabled.should_enable(None) == False + + def driver( + self, + data_dir, + monkeypatch, + django_settings_module, + basename_settings, + extra_settings_content, + expected, + ): + settings_content = ( + """ +# If the SECRET_KEY isn't defined we get the misleading error message +# CommandError: You must set settings.ALLOWED_HOSTS if DEBUG is False. +SECRET_KEY = "3*+d^_kjnr2gz)4q2m(&&^%$p4fj5dk3%lz4pl3g4m-%6!gf&)" + +# Must set ROOT_URLCONF else we get +# AttributeError: 'Settings' object has no attribute 'ROOT_URLCONF' +ROOT_URLCONF = "app.urls" + +MIDDLEWARE = ["appmap.django.Middleware"] + +""" + + extra_settings_content + + """ +""" + ) + monkeypatch.syspath_prepend(data_dir / "django") + monkeypatch.setenv("DJANGO_SETTINGS_MODULE", django_settings_module) + filename = data_dir / "django" / "app" / basename_settings + try: + with open(filename, "w") as f: + f.write(settings_content) + monkeypatch.chdir(str(data_dir / "django")) + assert DetectEnabled.should_enable("requests") == expected + finally: + file_delete(filename) + + def test__debug_false(self, data_dir, monkeypatch): + self.driver( + data_dir, + monkeypatch, + "app.settings_debug_false", + "settings_debug_false.py", + "DEBUG = False", + False, + ) + + def test__debug_true(self, data_dir, monkeypatch): + self.driver( + data_dir, + monkeypatch, + "app.settings_debug_true", + "settings_debug_true.py", + "DEBUG = True", + True, + ) + + # note this is APPMAP = False in the settings, not the env + def test__appmap_false(self, data_dir, monkeypatch): + self.driver( + data_dir, + monkeypatch, + "app.settings_appmap_false", + "settings_appmap_false.py", + "APPMAP = False", + False, + ) + + def test__appmap_true(self, data_dir, monkeypatch): + self.driver( + data_dir, + monkeypatch, + "app.settings_appmap_true", + "settings_appmap_true.py", + "APPMAP = True", + True, + ) diff --git a/appmap/test/test_django.py b/appmap/test/test_django.py index 2fb7f0b4..114615c2 100644 --- a/appmap/test/test_django.py +++ b/appmap/test/test_django.py @@ -39,7 +39,8 @@ @pytest.mark.django_db -def test_sql_capture(events): +def test_sql_capture(events, monkeypatch): + monkeypatch.setenv("APPMAP", "false") conn = django.db.connections["default"] conn.cursor().execute("SELECT 1").fetchall() assert events[0].sql_query == DictIncluding( @@ -52,7 +53,10 @@ def test_sql_capture(events): @pytest.mark.appmap_enabled -def test_framework_metadata(client, events): # pylint: disable=unused-argument +def test_framework_metadata( + client, events, monkeypatch +): # pylint: disable=unused-argument + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/") assert Metadata()["frameworks"] == [ {"name": "Django", "version": django.get_version()} @@ -60,13 +64,15 @@ def test_framework_metadata(client, events): # pylint: disable=unused-argument @pytest.mark.appmap_enabled -def test_app_can_read_body(client, events): # pylint: disable=unused-argument +def test_app_can_read_body(client, events, monkeypatch): # pylint: disable=unused-argument + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") response = client.post("/echo", json={"test": "json"}) assert response.content == b'{"test": "json"}' @pytest.mark.appmap_enabled -def test_template(events): +def test_template(events, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") render_to_string("hello_world.html") assert events[0].to_dict() == DictIncluding( { @@ -121,7 +127,8 @@ def client(): @pytest.mark.appmap_enabled -def test_unnamed(client, events): +def test_unnamed(client, events, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/post/unnamed/5") assert appmap.enabled() # sanity check @@ -131,7 +138,8 @@ def test_unnamed(client, events): @pytest.mark.appmap_enabled -def test_included_view(client, events): +def test_included_view(client, events, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/post/included/test_user") assert len(events) == 2 @@ -145,6 +153,8 @@ def test_included_view(client, events): @pytest.mark.appmap_enabled def test_exception(client, events, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") + def raise_on_call(*args): raise RuntimeError("An error") @@ -166,7 +176,8 @@ def raise_on_call(*args): @pytest.mark.appmap_enabled -def test_deeply_nested_routes(client, events): +def test_deeply_nested_routes(client, events, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/admincp/permissions/edit/1") assert len(events) == 2 @@ -194,30 +205,25 @@ def test_middleware_reset(pytester, monkeypatch): class TestRecordRequestsDjango(TestRecordRequests): @staticmethod - def setup_class(): - TestRecordRequestsDjango.server_stop() # ensure it's not running - TestRecordRequestsDjango.server_start() - - @staticmethod - def teardown_class(): - TestRecordRequestsDjango.server_stop() - - @staticmethod - def server_start_thread(): + def server_start_thread(env_vars_str): exec_cmd( """ # use appmap from our working copy, not the module installed by virtualenv export PYTHONPATH=`pwd` cd appmap/test/data/django/ -APPMAP=true APPMAP_RECORD_REQUESTS=true APPMAP_OUTPUT_DIR=/tmp python manage.py runserver 127.0.0.1:""" +""" + + env_vars_str + + """ APPMAP_OUTPUT_DIR=/tmp python manage.py runserver 127.0.0.1:""" + str(TestRecordRequests.server_port) ) @staticmethod - def server_start(): + def server_start(env_vars_str): # start as background thread so running the tests can continue - thread = Thread(target=TestRecordRequestsDjango.server_start_thread) + thread = Thread( + target=TestRecordRequestsDjango.server_start_thread, args=(env_vars_str,) + ) thread.start() wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "open") @@ -228,8 +234,27 @@ def server_stop(): ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") - def test_record_request_no_remote(client, events): + def test_record_request_appmap_enabled_requests_enabled_no_remote(client, events): + TestRecordRequestsDjango.server_stop() # ensure it's not running + TestRecordRequestsDjango.server_start("APPMAP=true APPMAP_RECORD_REQUESTS=true") TestRecordRequests.record_request(client, events, False) + TestRecordRequestsDjango.server_stop() - def test_record_request_and_remote(client, events): + def test_record_request_appmap_enabled_requests_enabled_and_remote(client, events): + TestRecordRequestsDjango.server_stop() # ensure it's not running + TestRecordRequestsDjango.server_start("APPMAP=true APPMAP_RECORD_REQUESTS=true") TestRecordRequests.record_request(client, events, True) + TestRecordRequestsDjango.server_stop() + + # not enabled means APPMAP isn't set. This isn't the same as APPMAP=false. + def test_record_request_appmap_not_enabled_requests_enabled_no_remote( + client, events + ): + TestRecordRequestsDjango.server_stop() # ensure it's not running + TestRecordRequestsDjango.server_start("APPMAP_RECORD_REQUESTS=true") + TestRecordRequests.record_request(client, events, False) + TestRecordRequestsDjango.server_stop() + + # it's not possible to test for + # appmap_not_enabled_requests_enabled_and_remote because when + # APPMAP=false the routes for remote recording are disabled. diff --git a/appmap/test/test_flask.py b/appmap/test/test_flask.py index 101741ec..1e327b65 100644 --- a/appmap/test/test_flask.py +++ b/appmap/test/test_flask.py @@ -39,7 +39,10 @@ def flask_app(data_dir, monkeypatch): @pytest.mark.appmap_enabled -def test_framework_metadata(client, events): # pylint: disable=unused-argument +def test_framework_metadata( + client, events, monkeypatch +): # pylint: disable=unused-argument + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/") assert Metadata()["frameworks"] == [{"name": "flask", "version": flask.__version__}] @@ -61,30 +64,25 @@ def test_template(app, events): class TestRecordRequestsFlask(TestRecordRequests): @staticmethod - def setup_class(): - TestRecordRequestsFlask.server_stop() # ensure it's not running - TestRecordRequestsFlask.server_start() - - @staticmethod - def teardown_class(): - TestRecordRequestsFlask.server_stop() - - @staticmethod - def server_start_thread(): + def server_start_thread(env_vars_str): exec_cmd( """ # use appmap from our working copy, not the module installed by virtualenv export PYTHONPATH=`pwd` cd appmap/test/data/flask/ -APPMAP=true APPMAP_RECORD_REQUESTS=true APPMAP_OUTPUT_DIR=/tmp FLASK_DEBUG=1 FLASK_APP=app.py flask run -p """ +""" + + env_vars_str + + """ APPMAP_OUTPUT_DIR=/tmp FLASK_DEBUG=1 FLASK_APP=app.py flask run -p """ + str(TestRecordRequests.server_port) ) @staticmethod - def server_start(): + def server_start(env_vars_str): # start as background thread so running the tests can continue - thread = Thread(target=TestRecordRequestsFlask.server_start_thread) + thread = Thread( + target=TestRecordRequestsFlask.server_start_thread, args=(env_vars_str,) + ) thread.start() wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "open") @@ -95,8 +93,27 @@ def server_stop(): ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") - def test_record_request_no_remote(client, events): + def test_record_request_appmap_enabled_requests_enabled_no_remote(client, events): + TestRecordRequestsFlask.server_stop() # ensure it's not running + TestRecordRequestsFlask.server_start("APPMAP=true APPMAP_RECORD_REQUESTS=true") TestRecordRequests.record_request(client, events, False) + TestRecordRequestsFlask.server_stop() - def test_record_request_and_remote(client, events): + def test_record_request_appmap_enabled_requests_enabled_and_remote(client, events): + TestRecordRequestsFlask.server_stop() # ensure it's not running + TestRecordRequestsFlask.server_start("APPMAP=true APPMAP_RECORD_REQUESTS=true") TestRecordRequests.record_request(client, events, True) + TestRecordRequestsFlask.server_stop() + + # not enabled means APPMAP isn't set. This isn't the same as APPMAP=false. + def test_record_request_appmap_not_enabled_requests_enabled_no_remote( + client, events + ): + TestRecordRequestsFlask.server_stop() # ensure it's not running + TestRecordRequestsFlask.server_start("APPMAP_RECORD_REQUESTS=true") + TestRecordRequests.record_request(client, events, False) + TestRecordRequestsFlask.server_stop() + + # it's not possible to test for + # appmap_not_enabled_requests_enabled_and_remote because when + # APPMAP=false the routes for remote recording are disabled. diff --git a/appmap/test/test_test_frameworks.py b/appmap/test/test_test_frameworks.py index 8721b34f..77d70b19 100644 --- a/appmap/test/test_test_frameworks.py +++ b/appmap/test/test_test_frameworks.py @@ -29,7 +29,8 @@ def test_appmap_unittest_runner(testdir): def test_appmap_unittest_runner_disabled(testdir, monkeypatch): - monkeypatch.delenv("APPMAP", raising=False) + # since not setting APPMAP records by default, must disable it explicitly + monkeypatch.setenv("APPMAP", "false") result = testdir.run( sys.executable, diff --git a/appmap/test/web_framework.py b/appmap/test/web_framework.py index 155d6384..0a4490d8 100644 --- a/appmap/test/web_framework.py +++ b/appmap/test/web_framework.py @@ -15,6 +15,7 @@ import requests import appmap +from appmap._implementation.detect_enabled import DetectEnabled from appmap.test.helpers import DictIncluding from .normalize import normalize_appmap @@ -33,8 +34,9 @@ class TestRequestCapture: """Common tests for HTTP server request and response capture.""" @staticmethod - def test_http_capture(client, events): + def test_http_capture(client, events, monkeypatch): """Test GET request and response capture.""" + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/test") assert events[0].http_server_request == DictIncluding( @@ -49,8 +51,9 @@ def test_http_capture(client, events): assert "ETag" in response["headers"] @staticmethod - def test_http_capture_post(client, events): + def test_http_capture_post(client, events, monkeypatch): """Test POST request with JSON body capture.""" + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post( "/test", json={"my_param": "example"}, @@ -76,7 +79,8 @@ def test_http_capture_post(client, events): ) @staticmethod - def test_post(events, client): + def test_post(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post( "/test", data=json.dumps({"my_param": "example"}), @@ -108,7 +112,8 @@ def test_post(events, client): ) @staticmethod - def test_get(events, client): + def test_get(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/test?my_param=example") assert events[0].message == [ @@ -118,7 +123,8 @@ def test_get(events, client): ] @staticmethod - def test_get_arr(events, client): + def test_get_arr(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/test?my_param=example&my_param=example2") assert events[0].message == [ @@ -132,7 +138,8 @@ def test_get_arr(events, client): ] @staticmethod - def test_post_form_urlencoded(events, client): + def test_post_form_urlencoded(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post( "/test", data="my_param=example", @@ -146,7 +153,8 @@ def test_post_form_urlencoded(events, client): ] @staticmethod - def test_put(events, client): + def test_put(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.put("/test", json={"my_param": "example"}) assert events[0].message == [ @@ -156,7 +164,8 @@ def test_put(events, client): ] @staticmethod - def test_post_bad_json(events, client): + def test_post_bad_json(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post( "/test?my_param=example", data="bad json", content_type="application/json" ) @@ -168,7 +177,8 @@ def test_post_bad_json(events, client): ] @staticmethod - def test_post_multipart(events, client): + def test_post_multipart(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post( "/test", data={"my_param": "example"}, content_type="multipart/form-data" ) @@ -180,7 +190,8 @@ def test_post_multipart(events, client): ] @staticmethod - def test_post_with_query(events, client): + def test_post_with_query(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.post("/test?my_param=get", data={"my_param": "example"}) assert events[0].message == [ @@ -202,14 +213,16 @@ def test_post_with_query(events, client): ("/post/test_user/123/summary", "/post/{username}/{post_id}/summary"), ], ) - def test_path_normalization(client, events, url, expected): + def test_path_normalization(client, events, url, expected, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get(url) np = events[0].http_server_request["normalized_path_info"] assert np == expected @staticmethod @pytest.mark.appmap_enabled - def test_message_path_segments(events, client): + def test_message_path_segments(events, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") client.get("/post/alice/42/summary") assert events[0].message == [ @@ -224,7 +237,9 @@ class TestRecording: """Common tests for remote recording.""" @staticmethod - def test_appmap_disabled(client): + def test_appmap_disabled(client, monkeypatch): + # since APPMAP records by default, disable it explicitly + monkeypatch.setenv("APPMAP", "false") assert not appmap.enabled() res = client.get("/_appmap/record") @@ -259,7 +274,8 @@ def test_can_only_enable_once(client): @staticmethod @pytest.mark.appmap_enabled - def test_can_record(data_dir, client): + def test_can_record(data_dir, client, monkeypatch): + monkeypatch.setenv("APPMAP_RECORD_REQUESTS", "false") res = client.post("/_appmap/record") assert res.status_code == 200 diff --git a/appmap/unittest.py b/appmap/unittest.py index 303c78a0..dec06922 100644 --- a/appmap/unittest.py +++ b/appmap/unittest.py @@ -5,14 +5,13 @@ import appmap import appmap.wrapt as wrapt from appmap._implementation import testing_framework +from appmap._implementation.detect_enabled import DetectEnabled logger = logging.getLogger(__name__) def setup_unittest(): - if not appmap.enabled(): - # Using this runner without enabling AppMap might not be what - # the user intended, so issue a warning. + if DetectEnabled.is_appmap_repo() or not DetectEnabled.should_enable("unittest"): logger.warning("AppMap disabled. Did you forget to set APPMAP=true?") return @@ -46,9 +45,13 @@ def _args(test_case, *_, isTest=False, **__): with session.record( test_case.__class__, method_name, location=location ) as metadata: - with wrapped(*args, **kwargs), testing_framework.collect_result_metadata( - metadata - ): + if metadata: + with wrapped( + *args, **kwargs + ), testing_framework.collect_result_metadata(metadata): + yield + else: + # session.record may return None yield