Skip to content

Commit f7c928e

Browse files
committed
ci(macrobenchmark): Fix app upload status and drop the iterations override
App Storage upload answers 201, not 200, so the strict status check rejected every upload before a device was even reserved. Also drop --iterations: androidx.benchmark.iterations only feeds the microbenchmark path (BenchmarkStateLegacy, MicrobenchmarkPhase). Macrobenchmark reads its iteration count from the test source and ignores the argument, so skipping AOT compilation is the only way to shorten a run that still writes results -- dryRunMode would shorten it but forces outputEnable to false, producing no benchmarkData.json to retrieve. Track installations by installationId rather than by the app reference, which Sauce may echo back normalised.
1 parent 619246e commit f7c928e

2 files changed

Lines changed: 30 additions & 23 deletions

File tree

‎.github/workflows/integration-tests-macrobenchmark.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ on:
1010
workflow_dispatch:
1111
inputs:
1212
mode:
13-
description: 'smoke = 1 iteration without AOT compilation (fast, validates the plumbing); full = the benchmark as declared'
13+
description: 'smoke = skip AOT compilation (faster, validates the plumbing); full = the benchmark as declared'
1414
type: choice
1515
options:
1616
- smoke
@@ -85,7 +85,7 @@ jobs:
8585
if: env.SAUCE_USERNAME != null
8686
run: |
8787
if [ "$MODE" = "smoke" ]; then
88-
mode_args=(--iterations 1 --skip-compilation)
88+
mode_args=(--skip-compilation)
8989
else
9090
mode_args=()
9191
fi

‎scripts/macrobenchmark-sauce.py‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
# Just check whether this account has Real Device Access API entitlement
2020
python3 scripts/macrobenchmark-sauce.py --probe-only
2121
22-
# Fast smoke test: 1 iteration, no AOT compilation
22+
# Faster smoke test: same iterations, but no AOT compilation
2323
python3 scripts/macrobenchmark-sauce.py \
2424
--app sentry-samples/sentry-samples-android/build/outputs/apk/release/sentry-samples-android-release.apk \
2525
--test-app sentry-android-integration-tests/sentry-uitest-android-macrobenchmark/build/outputs/apk/benchmark/sentry-uitest-android-macrobenchmark-benchmark.apk \
26-
--iterations 1 --skip-compilation
26+
--skip-compilation
2727
2828
# Full run, as the benchmark declares it
2929
python3 scripts/macrobenchmark-sauce.py --app <apk> --test-app <apk>
@@ -107,10 +107,13 @@ def upload_app(self, apk):
107107
response = self._request(
108108
"POST",
109109
f"{self.api}/v1/storage/upload",
110+
expect=(200, 201),
110111
files={"payload": (apk.name, payload)},
111112
data={"name": apk.name},
112113
)
113-
return "storage:" + response.json()["item"]["id"]
114+
reference = "storage:" + response.json()["item"]["id"]
115+
print(f"Uploaded {apk.name} as {reference}")
116+
return reference
114117

115118
# --- session lifecycle -------------------------------------------------
116119

@@ -163,14 +166,17 @@ def install(self, app_reference, timeout=600):
163166
# enableInstrumentation=false keeps the APKs byte-identical: Sauce's
164167
# instrumentation re-signs and hooks the app, which would mean benchmarking
165168
# something other than what we built. We need none of the features it unlocks.
166-
self._device(
169+
started = self._device(
167170
"installApp", json={"app": app_reference, "enableInstrumentation": False}
168-
)
171+
).json()
172+
# Track by installationId rather than the app reference, which Sauce may echo back
173+
# in a normalised form.
174+
installation_id = started["installationId"]
169175
deadline = time.monotonic() + timeout
170176
while time.monotonic() < deadline:
171177
installations = self._device("listAppInstallations").json()["appInstallations"]
172178
status = next(
173-
(i for i in installations if i.get("app") == app_reference), None
179+
(i for i in installations if i.get("installationId") == installation_id), None
174180
)
175181
if status and status["status"] == "FINISHED":
176182
print(f"Installed {app_reference}")
@@ -190,23 +196,25 @@ def pull_file(self, path):
190196
return self._device("pullFile", json={"path": path}).content
191197

192198

193-
def instrumentation_args(iterations, skip_compilation):
199+
def instrumentation_args(skip_compilation):
194200
"""Builds the `-e key value` arguments for `am instrument`.
195201
196-
Note we never pass androidx.benchmark.dryRunMode.enable: dry-run mode forces
197-
outputEnable to false (Arguments.kt), so it writes no benchmarkData.json at all
198-
and cannot verify that retrieval works. For a fast check use one iteration with
199-
compilation disabled instead -- that still produces real output.
202+
Deliberately not offered here:
203+
204+
- androidx.benchmark.dryRunMode.enable would cut the run to one iteration, but it
205+
also forces outputEnable to false (Arguments.kt), so no benchmarkData.json is
206+
written at all -- a dry run cannot verify that retrieval works.
207+
- androidx.benchmark.iterations only feeds the *micro*benchmark path
208+
(BenchmarkStateLegacy, MicrobenchmarkPhase). Macrobenchmark takes its iteration
209+
count from the test source and ignores the argument, so skipping AOT compilation
210+
is the only way to shorten a run while still producing results.
200211
"""
201-
args = []
202-
if iterations is not None:
203-
args += ["androidx.benchmark.iterations", str(iterations)]
204-
if skip_compilation:
205-
args += ["androidx.benchmark.compilation.enabled", "false"]
206-
return " ".join(f"-e {key} {value}" for key, value in zip(args[::2], args[1::2]))
212+
if not skip_compilation:
213+
return ""
214+
return "-e androidx.benchmark.compilation.enabled false"
207215

208216

209-
def run_benchmark(device, iterations, skip_compilation, timeout):
217+
def run_benchmark(device, skip_compilation, timeout):
210218
"""Starts the instrumentation detached and waits for it to finish."""
211219
# Leading rm is belt and braces -- Outputs also clears its dir on startup. Separated by
212220
# `;` so a missing dir on a fresh device does not stop the mkdir.
@@ -215,7 +223,7 @@ def run_benchmark(device, iterations, skip_compilation, timeout):
215223
stdout_file = f"{SHELL_SCRATCH_DIR}/instrumentation.txt"
216224
exit_code_file = f"{SHELL_SCRATCH_DIR}/exitcode"
217225
instrumentation = (
218-
f"am instrument -w -r {instrumentation_args(iterations, skip_compilation)} "
226+
f"am instrument -w -r {instrumentation_args(skip_compilation)} "
219227
f"{TEST_PACKAGE}/{TEST_RUNNER}"
220228
)
221229
# Detached, because executeShellCommand is documented to time out on long-running
@@ -293,7 +301,6 @@ def main():
293301
parser.add_argument("--test-app", type=Path, help="macrobenchmark instrumentation APK")
294302
parser.add_argument("--device-name", default=DEFAULT_DEVICE, help="device id or regex")
295303
parser.add_argument("--region", default="us-west-1")
296-
parser.add_argument("--iterations", type=int, help="override the benchmark's iteration count")
297304
parser.add_argument(
298305
"--skip-compilation",
299306
action="store_true",
@@ -338,7 +345,7 @@ def main():
338345
device.install(test_app_reference)
339346
device.disable_animations()
340347

341-
exit_code, _ = run_benchmark(device, args.iterations, args.skip_compilation, args.timeout)
348+
exit_code, _ = run_benchmark(device, args.skip_compilation, args.timeout)
342349
results = pull_results(device, args.out_dir)
343350
finally:
344351
device.close()

0 commit comments

Comments
 (0)