Skip to content

Commit 5e83545

Browse files
jpnurmiclaude
andauthored
test: stabilize flaky tests (getsentry#1614)
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 48550ae commit 5e83545

13 files changed

Lines changed: 100 additions & 113 deletions

.github/workflows/ci.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,11 @@ jobs:
364364

365365
- name: Install sccache
366366
if: ${{ runner.os == 'Windows' && env['USE_SCCACHE'] }}
367+
continue-on-error: true # build with plain ninja if chocolatey is down
367368
shell: bash
368-
run: choco install sccache -y
369+
run: |
370+
choco install sccache -y
371+
sccache --version
369372
370373
- name: Cache sccache
371374
if: ${{ runner.os == 'Windows' && env['USE_SCCACHE'] }}

tests/__init__.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@
1212

1313
sourcedir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
1414

15+
16+
def adb(*args, **kwargs):
17+
return subprocess.run(
18+
["{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]), *args], **kwargs
19+
)
20+
21+
1522
# https://docs.pytest.org/en/latest/assert.html#assert-details
1623
pytest.register_assert_rewrite("tests.assertions")
1724

tests/assertions.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@
55
import re
66
import sys
77
from dataclasses import dataclass
8-
from datetime import datetime, UTC
8+
from datetime import datetime, timedelta, UTC
99
from pathlib import Path
1010

11+
import tests
12+
1113
import msgpack
1214

1315
from . import SENTRY_VERSION
@@ -360,8 +362,10 @@ def assert_minidump(envelope):
360362

361363

362364
def assert_timestamp(ts):
363-
elapsed_time = datetime.now(UTC) - datetime.fromisoformat(ts)
364-
assert elapsed_time.total_seconds() < 10
365+
dt = datetime.fromisoformat(ts)
366+
# 1s tolerance for `date +%s` truncation in device clock offset measurement
367+
assert dt <= tests.now() + timedelta(seconds=1), "timestamp is in the future"
368+
assert dt >= tests.test_start, "timestamp is in the past"
365369

366370

367371
def assert_event(envelope, message="Hello World!", expected_trace_id=""):

tests/cmake.py

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
import pytest
1010

11+
from . import adb
12+
from .conditions import has_sccache
1113
from .build_config import (
1214
get_android_config,
1315
get_platform_cmake_args,
@@ -129,7 +131,7 @@ def cmake_configure(cwd, options, cflags=None):
129131
__tracebackhide__ = True
130132

131133
options = dict(options)
132-
if os.environ.get("USE_SCCACHE"):
134+
if has_sccache:
133135
options.update(
134136
{
135137
"CMAKE_C_COMPILER_LAUNCHER": "sccache",
@@ -233,7 +235,7 @@ def cmake_build(cwd, targets, options):
233235
"cmake",
234236
]
235237
env = dict(os.environ)
236-
if env.get("USE_SCCACHE"):
238+
if has_sccache:
237239
# Each pytest run builds in a new temp directory. Paths are normalized
238240
# relative to the build dir to allow sccache hits across runs.
239241
env.setdefault("SCCACHE_BASEDIRS", str(cwd))
@@ -308,16 +310,7 @@ def cmake_build(cwd, targets, options):
308310

309311
if os.environ.get("ANDROID_API"):
310312
# copy the output to the android image via adb
311-
subprocess.run(
312-
[
313-
"{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]),
314-
"push",
315-
"./",
316-
"/data/local/tmp",
317-
],
318-
cwd=cwd,
319-
check=True,
320-
)
313+
adb("push", "./", "/data/local/tmp", cwd=cwd, check=True)
321314

322315

323316
def configure_clang_cl(config_cmd: list[str]):

tests/conditions.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import sys
22
import os
3+
import shutil
34

45
is_aix = sys.platform == "aix" or sys.platform == "os400"
56
is_android = os.environ.get("ANDROID_API")
@@ -48,3 +49,5 @@
4849
# It's always available - tests explicitly set SENTRY_BACKEND: native in cmake
4950
# On macOS ASAN, the signal handling conflicts with ASAN's memory interception
5051
has_native = has_http and not (is_asan and sys.platform == "darwin")
52+
53+
has_sccache = os.environ.get("USE_SCCACHE") and shutil.which("sccache")

tests/conftest.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
import re
66
import statistics
77
import sys
8-
from . import run
8+
from datetime import datetime, timedelta, UTC
9+
from . import adb, run
910
from .cmake import CMake
11+
from .conditions import has_sccache, is_android
12+
import tests
1013

1114
LABEL = "label"
1215
TIME_UNIT = "time_unit"
@@ -41,6 +44,28 @@ def cmake(tmp_path_factory):
4144
cmake.destroy()
4245

4346

47+
def _get_clock_offset():
48+
"""Measure clock offset between host and Android device."""
49+
if not is_android:
50+
return timedelta(0)
51+
try:
52+
before = datetime.now(UTC)
53+
result = adb("shell", "date", "+%s", capture_output=True, text=True)
54+
after = datetime.now(UTC)
55+
device_time = datetime.fromtimestamp(int(result.stdout.strip()), tz=UTC)
56+
host_time = before + (after - before) / 2
57+
return device_time - host_time
58+
except (KeyError, ValueError, OSError):
59+
return timedelta(0)
60+
61+
62+
@pytest.fixture(autouse=True)
63+
def _record_test_start():
64+
offset = _get_clock_offset()
65+
tests.now = lambda: datetime.now(UTC) + offset
66+
tests.test_start = tests.now()
67+
68+
4469
def pytest_addoption(parser):
4570
parser.addoption(
4671
"--with_crashpad_wer",
@@ -143,12 +168,12 @@ def pytest_sessionfinish(session, exitstatus):
143168

144169

145170
def pytest_sessionstart(session):
146-
if os.environ.get("USE_SCCACHE"):
171+
if has_sccache:
147172
subprocess.run(["sccache", "--zero-stats"], capture_output=True)
148173

149174

150175
def pytest_terminal_summary(terminalreporter):
151-
if os.environ.get("USE_SCCACHE"):
176+
if has_sccache:
152177
result = subprocess.run(
153178
["sccache", "--show-stats"], capture_output=True, text=True
154179
)

tests/test_dotnet_signals.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import pytest
99

10+
from tests import adb
1011
from tests.conditions import is_android, is_tsan, is_x86, is_asan
1112

1213
project_fixture_path = pathlib.Path("tests/fixtures/dotnet_signal")
@@ -275,11 +276,6 @@ def wait_for(condition, timeout=10, interval=0.5):
275276
return condition()
276277

277278

278-
def adb(*args, **kwargs):
279-
adb_path = "{}/platform-tools/adb".format(os.environ["ANDROID_HOME"])
280-
return subprocess.run([adb_path, *args], **kwargs)
281-
282-
283279
def run_android(args=None, timeout=30):
284280
if args is None:
285281
args = []

tests/test_inproc_stress.py

Lines changed: 23 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -8,29 +8,16 @@
88

99
import pytest
1010

11-
from . import Envelope
11+
from . import adb, Envelope
1212
from .assertions import assert_inproc_crash
1313
from .build_config import get_test_executable_cmake_args, get_test_executable_env
14-
from .conditions import is_tsan
14+
from .conditions import is_android, is_tsan
1515

1616
fixture_path = pathlib.Path("tests/fixtures/inproc_stress")
1717

1818
ANDROID_TMP = "/data/local/tmp"
1919

2020

21-
def is_android():
22-
return bool(os.environ.get("ANDROID_API"))
23-
24-
25-
def adb(*args):
26-
"""Run an adb command."""
27-
return subprocess.run(
28-
["{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]), *args],
29-
check=True,
30-
capture_output=True,
31-
)
32-
33-
3421
def compile_test_program(tmp_path):
3522
build_dir = tmp_path / "inproc_stress_build"
3623
build_dir.mkdir(exist_ok=True)
@@ -63,14 +50,14 @@ def compile_test_program(tmp_path):
6350
exe_path = build_dir / exe_name
6451

6552
# Push executable to Android device
66-
if is_android():
67-
adb("push", str(exe_path), ANDROID_TMP)
53+
if is_android:
54+
adb("push", str(exe_path), ANDROID_TMP, check=True, capture_output=True)
6855

6956
return exe_path
7057

7158

7259
def run_stress_test(tmp_path, test_executable, test_name, database_path=None):
73-
if is_android():
60+
if is_android:
7461
return run_stress_test_android(test_executable, test_name, database_path)
7562

7663
if database_path is None:
@@ -100,20 +87,14 @@ def run_stress_test_android(test_executable, test_name, database_path):
10087
remote_db_path = f"{ANDROID_TMP}/{database_path.name}"
10188

10289
# Clear logcat before running so we limit the capture as close to this run as possible
103-
subprocess.run(
104-
["{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]), "logcat", "-c"],
105-
check=False,
106-
)
90+
adb("logcat", "-c", check=False)
10791

10892
# Run on device - we need to capture both stdout and stderr, and the return code
10993
# Android shell doesn't separate stdout/stderr well, so we redirect stderr to stdout
11094
# and parse the return code from the output (same approach as tests/__init__.py)
111-
result = subprocess.run(
112-
[
113-
"{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]),
114-
"shell",
115-
f"cd {ANDROID_TMP} && LD_LIBRARY_PATH=. ./{exe_name} {test_name} {remote_db_path} 2>&1; echo ret:$?",
116-
],
95+
result = adb(
96+
"shell",
97+
f"cd {ANDROID_TMP} && LD_LIBRARY_PATH=. ./{exe_name} {test_name} {remote_db_path} 2>&1; echo ret:$?",
11798
capture_output=True,
11899
text=True,
119100
)
@@ -130,14 +111,11 @@ def run_stress_test_android(test_executable, test_name, database_path):
130111
time.sleep(0.5)
131112

132113
# Capture logcat to get our logs
133-
logcat_result = subprocess.run(
134-
[
135-
"{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]),
136-
"logcat",
137-
"-d",
138-
"-s",
139-
"sentry-native:*",
140-
],
114+
logcat_result = adb(
115+
"logcat",
116+
"-d",
117+
"-s",
118+
"sentry-native:*",
141119
capture_output=True,
142120
text=True,
143121
)
@@ -150,20 +128,19 @@ def run_stress_test_android(test_executable, test_name, database_path):
150128

151129
# Pull the remote database to local path (pulls to parent, creates database_path)
152130
try:
153-
adb("pull", f"{remote_db_path}/", str(database_path.parent))
131+
adb(
132+
"pull",
133+
f"{remote_db_path}/",
134+
str(database_path.parent),
135+
check=True,
136+
capture_output=True,
137+
)
154138
except subprocess.CalledProcessError:
155139
# Database might not exist if crash wasn't captured
156140
pass
157141

158142
# Clean up remote database for next run
159-
subprocess.run(
160-
[
161-
"{}/platform-tools/adb".format(os.environ["ANDROID_HOME"]),
162-
"shell",
163-
f"rm -rf {remote_db_path}",
164-
],
165-
check=False,
166-
)
143+
adb("shell", f"rm -rf {remote_db_path}", check=False)
167144

168145
# Combine shell output with logcat output for assertion checks
169146
combined_output = output + "\n" + logcat_output
@@ -374,7 +351,7 @@ def test_inproc_handler_abort_crash(cmake):
374351

375352

376353
@pytest.mark.skipif(
377-
sys.platform != "darwin" or is_android(),
354+
sys.platform != "darwin" or bool(is_android),
378355
reason="Stack trace tests are macOS-only",
379356
)
380357
@pytest.mark.parametrize(

tests/test_integration_native.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -99,24 +99,26 @@ def test_native_oom(cmake, httpserver):
9999
"""Test OOM crash capture with native backend"""
100100
tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"})
101101

102-
httpserver.expect_request("/api/123456/envelope/").respond_with_data("OK")
102+
httpserver.expect_oneshot_request("/api/123456/envelope/").respond_with_data("OK")
103103

104-
run_crash(
105-
tmp_path,
106-
"sentry_example",
107-
["log", "stdout", "oom"],
108-
env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
109-
)
104+
with httpserver.wait(timeout=10) as waiting:
105+
run_crash(
106+
tmp_path,
107+
"sentry_example",
108+
["log", "stdout", "oom"],
109+
env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
110+
)
110111

111-
time.sleep(2)
112+
time.sleep(2)
112113

113-
run(
114-
tmp_path,
115-
"sentry_example",
116-
["log", "no-setup"],
117-
env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
118-
)
114+
run(
115+
tmp_path,
116+
"sentry_example",
117+
["log", "no-setup"],
118+
env=dict(os.environ, SENTRY_DSN=make_dsn(httpserver)),
119+
)
119120

121+
assert waiting.result
120122
assert len(httpserver.log) >= 1
121123

122124

0 commit comments

Comments
 (0)