Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: request recording in unittest setUp method #338

Merged
merged 1 commit into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions _appmap/test/data/django/test/test_unittest_setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@

from unittest import TestCase

from django.test import Client

class DisabledRequestsRecordingTest(TestCase):
def setUp(self) -> None:
Client().get("/")

def test_request_in_setup(self):
pass
10 changes: 6 additions & 4 deletions _appmap/test/test_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,21 @@ def test_enabled(self, pytester):
# To really check middleware reset, the tests must run in order,
# so disable randomly.
result = pytester.runpytest("-svv", "-p", "no:randomly")
result.assert_outcomes(passed=4, failed=0, errors=0)
result.assert_outcomes(passed=5, failed=0, errors=0)
# Look for the http_server_request event in test_app's appmap. If
# middleware reset is broken, it won't be there.
appmap_file = pytester.path / "tmp" / "appmap" / "pytest" / "test_request.appmap.json"
assert not os.path.exists(pytester.path / "tmp" / "appmap" / "requests")
assert not os.path.exists(
pytester.path / "tmp" / "appmap" / "requests"
), "django tests generated request recordings"

events = json.loads(appmap_file.read_text())["events"]
assert "http_server_request" in events[0]

def test_disabled(self, pytester, monkeypatch):
monkeypatch.setenv("_APPMAP", "false")
result = pytester.runpytest("-svv", "-p", "no:randomly", "-k", "test_request")
result.assert_outcomes(passed=1, failed=0, errors=0)
result.assert_outcomes(passed=2, failed=0, errors=0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs to check that no request recordings were created.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the following line assert that as well?

assert not (pytester.path / "tmp").exists()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it does, in the case that APPMAP=false (i.e. recording is completely disabled). The test I asked for ensures that request recordings aren't generated when other AppMaps are.

assert not (pytester.path / "tmp").exists()

def test_disabled_for_process(self, pytester, monkeypatch):
Expand All @@ -223,7 +225,7 @@ def test_disabled_for_process(self, pytester, monkeypatch):

# There are two tests for remote recording. They should both fail,
# because process recording should disable remote recording.
result.assert_outcomes(passed=2, failed=2, errors=0)
result.assert_outcomes(passed=3, failed=2, errors=0)

assert (pytester.path / "tmp" / "appmap" / "process").exists()
assert not (pytester.path / "tmp" / "appmap" / "requests").exists()
Expand Down
12 changes: 12 additions & 0 deletions _appmap/unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from contextlib import contextmanager

from _appmap import noappmap, testing_framework, wrapt
from _appmap.env import Env
from _appmap.utils import get_function_location

_session = testing_framework.session("unittest", "tests")
Expand Down Expand Up @@ -52,6 +53,17 @@ def _args(test_case, *_, isTest=False, **__):
yield

else:
# We need to disable request recording in TestCase._callSetUp too
# in order to prevent creation of a request recording besides test
# recording when requests are made inside setUp method.
# This edge case can be observed in this test in django project:
# $ APPMAP=TRUE ./runtests.py auth_tests.test_views.ChangelistTests.test_user_change_email
#  (ChangelistTests.setUp makes a request)
@wrapt.patch_function_wrapper("unittest.case", "TestCase._callSetUp")
def callSetUp(wrapped, test_case, args, kwargs): # pylint: disable=unused-argument
with Env.current.disabled("requests"):
wrapped(*args, **kwargs)

# As of 3.8, unittest.case.TestCase now calls the test's method indirectly, through
# TestCase._callTestMethod. Hook that to manage a recording session.
@wrapt.patch_function_wrapper("unittest.case", "TestCase._callTestMethod")
Expand Down
Loading