Skip to content

Commit b180f90

Browse files
committed
refactor: move write_appmap out of web_framework
It never made any sense to have write_appmap in web_framework. Move it to recording where it belongs.
1 parent 87efc91 commit b180f90

File tree

4 files changed

+65
-43
lines changed

4 files changed

+65
-43
lines changed

_appmap/recording.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,27 @@ def __exit__(self, exc_type, exc_value, tb):
4444
if self.exit_hook is not None:
4545
self.exit_hook(self)
4646
return False
47+
48+
49+
def write_appmap(
50+
appmap, appmap_fname, recorder_type, metadata=None, basedir=Env.current.output_dir
51+
):
52+
"""Write an appmap file into basedir.
53+
54+
Adds APPMAP_SUFFIX to basename; shortens the name if necessary.
55+
Atomically replaces existing files. Creates the basedir if required.
56+
"""
57+
58+
if len(appmap_fname) > NAME_MAX - len(APPMAP_SUFFIX):
59+
part = NAME_MAX - len(APPMAP_SUFFIX) - 1 - HASH_LEN
60+
appmap_fname = appmap_fname[:part] + "-" + name_hash(appmap_fname[part:])[:HASH_LEN]
61+
filename = appmap_fname + APPMAP_SUFFIX
62+
63+
basedir = basedir / recorder_type
64+
basedir.mkdir(parents=True, exist_ok=True)
65+
66+
with NamedTemporaryFile(mode="w", dir=basedir, delete=False) as tmp:
67+
tmp.write(generation.dump(appmap, metadata))
68+
appmap_file = basedir / filename
69+
logger.info("info, writing %s", appmap_file)
70+
os.replace(tmp.name, appmap_file)

_appmap/test/test_test_frameworks.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
"""Test cases dealing with various test framework runners and cases."""
2+
23
# pylint: disable=missing-function-docstring
34

45
import json
56
import re
67
import sys
8+
import types
79
from abc import ABC, abstractmethod
10+
from pathlib import Path
811

912
import pytest
1013

11-
from _appmap import web_framework
14+
from _appmap import recording
1215

1316
from ..test.helpers import DictIncluding
1417
from .normalize import normalize_appmap
@@ -117,21 +120,33 @@ def test_pytest_trial(self, testdir):
117120
verify_expected_appmap(testdir)
118121

119122

120-
def test_overwrites_existing(tmp_path):
121-
foo_file = tmp_path / "foo.appmap.json"
123+
EMPTY_APPMAP = types.SimpleNamespace(events=[])
124+
125+
RECORDER_TYPE = "test"
126+
127+
128+
@pytest.fixture
129+
def recorder_outdir(tmp_path) -> Path:
130+
ret = tmp_path / RECORDER_TYPE
131+
ret.mkdir(parents=True)
132+
return ret
133+
134+
135+
def test_overwrites_existing(recorder_outdir):
136+
foo_file = recorder_outdir / "foo.appmap.json"
122137
foo_file.write_text("existing")
123-
web_framework.write_appmap(tmp_path, "foo", "replacement")
124-
assert foo_file.read_text() == "replacement"
138+
recording.write_appmap(EMPTY_APPMAP, "foo", RECORDER_TYPE, None, recorder_outdir.parent)
139+
assert foo_file.read_text().startswith('{"version"')
125140

126141

127-
def test_write_appmap(tmp_path):
128-
web_framework.write_appmap(tmp_path, "foo", "bar")
129-
assert (tmp_path / "foo.appmap.json").read_text() == "bar"
142+
def test_write_appmap(recorder_outdir):
143+
recording.write_appmap(EMPTY_APPMAP, "foo", RECORDER_TYPE, None, recorder_outdir.parent)
144+
assert (recorder_outdir / "foo.appmap.json").read_text().startswith('{"version"')
130145

131146
longname = "-".join(["testing"] * 42)
132-
web_framework.write_appmap(tmp_path, longname, "bar")
147+
recording.write_appmap(EMPTY_APPMAP, longname, RECORDER_TYPE, None, recorder_outdir.parent)
133148
expected_shortname = longname[:235] + "-5d6e10d.appmap.json"
134-
assert (tmp_path / expected_shortname).read_text() == "bar"
149+
assert (recorder_outdir / expected_shortname).read_text().startswith('{"version"')
135150

136151

137152
@pytest.fixture(name="testdir")

_appmap/testing_framework.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
"""Shared infrastructure for testing framework integration."""
22

33
import os
4-
from pathlib import PurePath
54
import re
6-
from contextlib import contextmanager
75
import traceback
6+
from contextlib import contextmanager
7+
from pathlib import PurePath
88

99
import inflection
1010

11-
from _appmap import configuration, env, generation, web_framework
11+
from _appmap import configuration, env, generation, recording
1212
from _appmap.env import Env
1313
from _appmap.recording import Recording
1414
from _appmap.utils import fqname, root_relative_path
@@ -121,8 +121,7 @@ def record(self, klass, method, **kwds):
121121
with rec, environ.disabled("requests"):
122122
yield metadata
123123
finally:
124-
basedir = environ.output_dir / self.name
125-
web_framework.write_appmap(basedir, item.filename, generation.dump(rec, metadata))
124+
recording.write_appmap(rec, item.filename, self.name, metadata)
126125

127126

128127
@contextmanager
@@ -135,7 +134,10 @@ def collect_result_metadata(metadata):
135134
yield
136135
metadata["test_status"] = "succeeded"
137136
except Exception as exn:
138-
metadata["test_failure"] = {"message": failure_message(exn), "location": failure_location(exn)}
137+
metadata["test_failure"] = {
138+
"message": failure_message(exn),
139+
"location": failure_location(exn),
140+
}
139141
metadata["test_status"] = "failed"
140142
metadata["exception"] = {"class": exn.__class__.__name__, "message": str(exn)}
141143
raise
@@ -149,7 +151,7 @@ def file_delete(filename):
149151

150152

151153
def failure_message(exn: Exception) -> str:
152-
return f'{exn.__class__.__name__}: {exn}'
154+
return f"{exn.__class__.__name__}: {exn}"
153155

154156

155157
def failure_location(exn: Exception) -> str:

_appmap/web_framework.py

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
from contextvars import ContextVar
1212
from hashlib import sha256
1313
from json.decoder import JSONDecodeError
14-
from tempfile import NamedTemporaryFile
14+
15+
from _appmap import recording
1516

1617
from . import generation, remote_recording
1718
from .env import Env
@@ -87,26 +88,6 @@ def name_hash(namepart):
8788
return sha256(os.fsencode(namepart)).hexdigest()
8889

8990

90-
def write_appmap(basedir, basename, contents):
91-
"""Write an appmap file into basedir.
92-
93-
Adds APPMAP_SUFFIX to basename; shortens the name if necessary.
94-
Atomically replaces existing files. Creates the basedir if required.
95-
"""
96-
97-
if len(basename) > NAME_MAX - len(APPMAP_SUFFIX):
98-
part = NAME_MAX - len(APPMAP_SUFFIX) - 1 - HASH_LEN
99-
basename = basename[:part] + "-" + name_hash(basename[part:])[:HASH_LEN]
100-
filename = basename + APPMAP_SUFFIX
101-
102-
if not basedir.exists():
103-
basedir.mkdir(parents=True, exist_ok=True)
104-
105-
with NamedTemporaryFile(mode="w", dir=basedir, delete=False) as tmp:
106-
tmp.write(contents)
107-
os.replace(tmp.name, basedir / filename)
108-
109-
11091
def create_appmap_file(
11192
output_dir,
11293
request_method,
@@ -126,16 +107,16 @@ def create_appmap_file(
126107
+ ") - "
127108
+ start_time.strftime("%T.%f")[:-3]
128109
)
129-
appmap_basename = scenario_filename(
130-
"_".join([str(start_time.timestamp()), request_full_path])
131-
)
110+
appmap_basename = scenario_filename("_".join([str(start_time.timestamp()), request_full_path]))
132111
appmap_file_path = os.path.join(output_dir, appmap_basename)
112+
113+
recorder_type = "requests"
133114
metadata = {
134115
"name": appmap_name,
135116
"timestamp": start_time.timestamp(),
136-
"recorder": {"name": "record_requests"},
117+
"recorder": {"name": "record_requests", "type": recorder_type},
137118
}
138-
write_appmap(output_dir, appmap_basename, generation.dump(rec, metadata))
119+
recording.write_appmap(rec, appmap_basename, recorder_type, metadata)
139120
headers["AppMap-Name"] = os.path.abspath(appmap_name)
140121
headers["AppMap-File-Name"] = os.path.abspath(appmap_file_path) + APPMAP_SUFFIX
141122

0 commit comments

Comments
 (0)