From b73157508a5a1ceed1a553fcf91b41f1486e0908 Mon Sep 17 00:00:00 2001 From: symwell <111290954+symwell@users.noreply.github.com> Date: Thu, 29 Sep 2022 10:56:35 -0400 Subject: [PATCH] feat: Record by default --- appmap/_implementation/detect_enabled.py | 182 ++++++++++++++++++ appmap/_implementation/testing_framework.py | 18 +- appmap/_implementation/web_framework.py | 113 ++++++------ appmap/django.py | 12 +- appmap/flask.py | 25 ++- appmap/pytest.py | 4 +- appmap/test/conftest.py | 8 + appmap/test/test_detect_enabled.py | 194 ++++++++++++++++++++ appmap/test/test_django.py | 67 +++++-- appmap/test/test_flask.py | 67 +++++-- appmap/test/web_framework.py | 12 +- appmap/unittest.py | 17 +- 12 files changed, 597 insertions(+), 122 deletions(-) create mode 100644 appmap/_implementation/detect_enabled.py create mode 100644 appmap/test/test_detect_enabled.py diff --git a/appmap/_implementation/detect_enabled.py b/appmap/_implementation/detect_enabled.py new file mode 100644 index 00000000..529fc39d --- /dev/null +++ b/appmap/_implementation/detect_enabled.py @@ -0,0 +1,182 @@ +""" +Detect if AppMap is enabled. +""" + +import importlib +import logging +import os +from textwrap import dedent + +logger = logging.getLogger(__name__) + + +class UnrecognizedRecordingMethodException(Exception): + pass + + +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/_implementation/event.py") and os.path.exists( + "appmap/_implementation/recording.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: + 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 Exception as exn: + # it wasn't set. it's ok + pass + + if settings: + try: + 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 Exception as exn: + # it wasn't set. it's ok + pass + + return ["it's not Django", None] diff --git a/appmap/_implementation/testing_framework.py b/appmap/_implementation/testing_framework.py index 3126689e..325fb549 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 @@ -104,10 +105,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) @@ -147,3 +144,16 @@ def collect_result_metadata(metadata): metadata["test_status"] = "failed" metadata["exception"] = {"class": exn.__class__.__name__, "message": str(exn)} raise + + +def file_write(filename, contents): + f = open(filename, "w") + f.write(contents) + f.close() + + +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 f8984ffd..7167b4d0 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.recording import Recorder @@ -130,35 +131,33 @@ def create_appmap_file( class AppmapMiddleware: - 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" + DetectEnabled.initialize() + + 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() - else: - # b) or c) - rec = Recorder(_EventIds.get_thread_id()) - 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. - - if rec.enabled: - start, call_event_id = self.before_request_main(rec, request) + if DetectEnabled.should_enable("requests"): + # a) requests + rec = Recorder(_EventIds.get_thread_id()) + 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() + + if rec and rec.enabled: + start, call_event_id = self.before_request_main(rec, request) return rec, start, call_event_id @@ -170,7 +169,6 @@ def after_request_hook( self, request, request_path, - record_url, recording_is_running, request_method, request_base_url, @@ -179,44 +177,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() + if DetectEnabled.should_enable("requests"): + # a) requests + rec = Recorder(_EventIds.get_thread_id()) + # 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.enabled: self.after_request_main(rec, response, start, call_event_id) - else: - # b) or c) - rec = Recorder(_EventIds.get_thread_id()) - # 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.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() + if rec.enabled: + self.after_request_main(rec, response, start, call_event_id) return response diff --git a/appmap/django.py b/appmap/django.py index 146ada28..c3866ed3 100644 --- a/appmap/django.py +++ b/appmap/django.py @@ -23,6 +23,7 @@ from django.urls.resolvers import _route_to_regex from appmap._implementation import generation, recording, web_framework +from appmap._implementation.detect_enabled import DetectEnabled from appmap._implementation.env import Env from appmap._implementation.event import ( ExceptionEvent, @@ -211,19 +212,25 @@ def normalize_path_info(path_info, resolved): class Middleware(AppmapMiddleware): def __init__(self, get_response): + super().__init__() self.get_response = get_response self.recorder = Recorder() - self.record_url = "/_appmap/record" def __call__(self, request): if not Env.current.enabled: return self.get_response(request) + if not ( + DetectEnabled.should_enable("remote") + or DetectEnabled.should_enable("requests") + ): + 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.enabled + request, request.path_info, self.recorder.enabled ) try: @@ -242,7 +249,6 @@ def __call__(self, request): self.after_request_hook( request, request.path_info, - self.record_url, self.recorder.enabled, request.method, request.build_absolute_uri(), diff --git a/appmap/flask.py b/appmap/flask.py index f09a570d..90400ac7 100644 --- a/appmap/flask.py +++ b/appmap/flask.py @@ -12,6 +12,7 @@ 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.recording import Recorder, Recording @@ -46,6 +47,7 @@ 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) @@ -54,13 +56,17 @@ def init_app(self, app): if not Env.current.enabled: return + if not ( + DetectEnabled.should_enable("remote") + or DetectEnabled.should_enable("requests") + ): + return + if not app: return self.recording = Recording() - self.record_url = "/_appmap/record" - # print('in init_app') app.add_url_rule( self.record_url, @@ -103,11 +109,14 @@ def record_delete(self): return json.loads(generation.dump(self.recording)) def before_request(self): - if not Env.current.enabled: + if not ( + DetectEnabled.should_enable("remote") + or DetectEnabled.should_enable("requests") + ): return rec, start, call_event_id = self.before_request_hook( - request, request.path, self.record_url, self.recording.is_running() + request, request.path, self.recording.is_running() ) def before_request_main(self, rec, request): @@ -140,13 +149,15 @@ def before_request_main(self, rec, request): return None, None def after_request(self, response): - if not Env.current.enabled: - return + if not ( + DetectEnabled.should_enable("remote") + or DetectEnabled.should_enable("requests") + ): + return response return self.after_request_hook( request, request.path, - self.record_url, self.recording.is_running(), request.method, request.base_url, diff --git a/appmap/pytest.py b/appmap/pytest.py index 6f941db5..4aabf620 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,8 @@ def __call__(self, wrapped, _, args, kwargs): return wrapped(*args, **kwargs) -if appmap.enabled(): +DetectEnabled.initialize() +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 51492177..a5d5187d 100644 --- a/appmap/test/conftest.py +++ b/appmap/test/conftest.py @@ -91,3 +91,11 @@ 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..2374356f --- /dev/null +++ b/appmap/test/test_detect_enabled.py @@ -0,0 +1,194 @@ +""" +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, file_write + + +class TestDetectEnabled: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + @patch.dict(os.environ, {"APPMAP": "false"}) + def test_none__appmap_disabled(self): + DetectEnabled.initialize() + 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 + DetectEnabled.initialize() + assert DetectEnabled.should_enable(None) == False + + def test_invalid__no_envvar(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable("invalid_recording_method") == False + + @patch.dict(os.environ, {"APPMAP": "true"}) + def test_invalid__appmap_enabled(self): + DetectEnabled.initialize() + 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: + DetectEnabled.initialize() + 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: + DetectEnabled.initialize() + 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: + DetectEnabled.initialize() + 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: + DetectEnabled.initialize() + assert DetectEnabled.should_enable(recording_method) == False + + +class TestDetectEnabledFlask: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + def test__none(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable(None) == False + + @patch.dict(os.environ, {"FLASK_APP": "app.py"}) + def test__flask_app(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable(None) == False + + @patch.dict(os.environ, {"FLASK_DEBUG": "0"}) + def test__flask_debug_0(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable("requests") == False + + @patch.dict(os.environ, {"FLASK_DEBUG": "1"}) + def test__flask_debug_1(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable("requests") == True + + @patch.dict(os.environ, {"FLASK_ENV": "production"}) + def test__flask_env_production(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable("requests") == False + + @patch.dict(os.environ, {"FLASK_ENV": "development"}) + def test__flask_env_development(self): + DetectEnabled.initialize() + assert DetectEnabled.should_enable("requests") == True + + +class TestDetectEnabledDjango: + @staticmethod + def setup_method(): + DetectEnabled.clear_cache() + + def test__none(self): + DetectEnabled.initialize() + 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: + file_write(str(filename), settings_content) + monkeypatch.chdir(str(data_dir / "django")) + DetectEnabled.initialize() + 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 69d434f5..6c402e44 100644 --- a/appmap/test/test_django.py +++ b/appmap/test/test_django.py @@ -52,7 +52,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()} @@ -194,30 +197,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 python manage.py runserver 127.0.0.1:""" +""" + + env_vars_str + + """ 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,10 +226,43 @@ def server_stop(): ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") - @pytest.mark.skipif(True, reason="don't pass until _EventIds stops producing duplicate ids") - def test_record_request_no_remote(client, events): + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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() - @pytest.mark.skipif(True, reason="don't pass until _EventIds stops producing duplicate ids") - def test_record_request_and_remote(client, events): + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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. + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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() + + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + def test_record_request_appmap_not_enabled_requests_enabled_and_remote( + client, events + ): + TestRecordRequestsDjango.server_stop() # ensure it's not running + TestRecordRequestsDjango.server_start("APPMAP_RECORD_REQUESTS=true") + TestRecordRequests.record_request(client, events, True) + TestRecordRequestsDjango.server_stop() diff --git a/appmap/test/test_flask.py b/appmap/test/test_flask.py index e72af4e1..bd1b8879 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 FLASK_DEBUG=1 FLASK_APP=app.py flask run -p """ +""" + + env_vars_str + + """ 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,10 +93,43 @@ def server_stop(): ) wait_until_port_is("127.0.0.1", TestRecordRequests.server_port, "closed") - @pytest.mark.skipif(True, reason="don't pass until _EventIds stops producing duplicate ids") - def test_record_request_no_remote(client, events): + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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() - @pytest.mark.skipif(True, reason="don't pass until _EventIds stops producing duplicate ids") - def test_record_request_and_remote(client, events): + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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. + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + 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() + + @pytest.mark.skipif( + True, reason="don't pass until _EventIds stops producing duplicate ids" + ) + def test_record_request_appmap_not_enabled_requests_enabled_and_remote( + client, events + ): + TestRecordRequestsFlask.server_stop() # ensure it's not running + TestRecordRequestsFlask.server_start("APPMAP_RECORD_REQUESTS=true") + TestRecordRequests.record_request(client, events, True) + TestRecordRequestsFlask.server_stop() diff --git a/appmap/test/web_framework.py b/appmap/test/web_framework.py index 71731ae8..62efb32d 100644 --- a/appmap/test/web_framework.py +++ b/appmap/test/web_framework.py @@ -353,7 +353,6 @@ def record_request( client, events, record_remote ): # pylint: disable=unused-argument - if record_remote: # when remote recording is enabled, this test also # verifies the global recorder doesn't save duplicate @@ -397,9 +396,14 @@ def record_request( assert response["AppMap-File-Name"] appmap_file_name = response["AppMap-File-Name"] assert exists(appmap_file_name) - appmap_file_name_basename = appmap_file_name.split('/')[-1] - appmap_file_name_basename_part = '_'.join(appmap_file_name_basename.split('_')[2:]) - assert appmap_file_name_basename_part == 'http_127_0_0_1_8000_test.appmap.json' + appmap_file_name_basename = appmap_file_name.split("/")[-1] + appmap_file_name_basename_part = "_".join( + appmap_file_name_basename.split("_")[2:] + ) + assert ( + appmap_file_name_basename_part + == "http_127_0_0_1_8000_test.appmap.json" + ) with open(appmap_file_name) as f: appmap = json.loads(f.read()) diff --git a/appmap/unittest.py b/appmap/unittest.py index 303c78a0..d2ffd33c 100644 --- a/appmap/unittest.py +++ b/appmap/unittest.py @@ -5,15 +5,14 @@ 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. - logger.warning("AppMap disabled. Did you forget to set APPMAP=true?") + DetectEnabled.initialize() + if DetectEnabled.is_appmap_repo() or not DetectEnabled.should_enable("unittest"): return session = testing_framework.session("unittest", "tests") @@ -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