feat: persist probe history#196
Conversation
piotr-iohk
left a comment
There was a problem hiding this comment.
Nice foundation for durable probe history. The probe-history/<run-id>/ contract + probe-run.json metadata look good, and clearing app logs before the Android run makes collected logs match that run.
This lines up with the storage half of synonymdev/bitkit-nightly#30. A couple of doc/contract issues should be fixed before merge.
Request changes
-
Docs reference a missing script
docs/mainnet-probe.mddocuments:node scripts/render-probe-trends.mjs
That script is not in this PR. Please either add it, or remove/adjust that section so we don’t document a command that doesn’t exist.
-
Keep
probe-history/compact
Copying Androidapp-logs/intoprobe-history/<run-id>/is fine for short-lived Actions artifacts, but nightly treats this tree as the long-term history contract and will commit it to theprobe-historybranch. App logs are ~1–2 MB per wallet per run and will bloat that branch quickly.
Prefer either:- omit
app-logs/fromprobe-history/(keep them only under top-levelartifacts/app-logs/), or - document clearly that consumers must strip
app-logs/before persisting to git, and have nightly do that.
- omit
CI compatibility (ci_run_android.sh)
Checked call sites in bitkit-android (e2e.yml, e2e_migration.yml), bitkit-e2e-tests migration setup, and nightly/probe workflows.
The new helpers look safe for non-probe E2E:
clear_app_logsfailures are non-fatal (important when the app isn’t installed yet at script start).copy_app_logs_to_probe_historyis a no-op unlessprobe-history/exists, which regular E2E specs don’t create.- Per-attempt artifact dirs (
ATTEMPT) mean the newrm -rfonapp-logswon’t clobber other attempts.
Only expected side effect: a benign “Could not clear app logs …” warning on fresh emulators before first install. Not a blocker.
Happy to approve after the docs + history-contract fix.
Summary
probe-history/<run-id>/.Testing
bash -n ci_run_android.sh.npx eslint test/helpers/probe.ts.git diff --check.