Skip to content

Daily smoke benchmark ci#440

Merged
azuchi merged 6 commits into
chaintope:masterfrom
Naviabheeman:DailySmokeBenchmarkCI
Jul 14, 2026
Merged

Daily smoke benchmark ci#440
azuchi merged 6 commits into
chaintope:masterfrom
Naviabheeman:DailySmokeBenchmarkCI

Conversation

@Naviabheeman

Copy link
Copy Markdown
Contributor

Improvements to CI:
smoke test - run jobs in parallel
CI on PR - do not run benchmark test to save time
Daily CI - run benchmark. plot a graph using the benchmark data using matplotlib python library

Naviabheeman and others added 5 commits July 8, 2026 19:37
smoke-test.yml ran the 124-script BASE_SCRIPTS functional suite at
-j 1 (fully sequential) across all 5 platform configs on every push/PR.
Reuse the same $CPU_COUNT already computed for the build/ctest steps in
this job instead.

Also drop -DENABLE_BENCH=ON: smoke test built bench_tapyrus but never
ran it, and the corresponding bench_results.txt artifact-upload entry
was dead (never produced here, silently no-op via if-no-files-found:
ignore). Removes wasted build time for a binary nothing uses.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
daily-test.yml triggers on both schedule and pull_request, so every PR
was already paying for the full 13-config functional-test matrix plus
the benchmark step and its history-cache round-trip. Benchmarks add
real time and don't need to run per-PR - gate the benchmark execution,
and its two history-cache steps, behind github.event_name != 'pull_request'.
The functional-test matrix itself is untouched and still runs on every PR.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No structured CI reporting existed - test_runner.py only prints to
console and dumps logs into an uploaded artifact directory, so triaging
a failure meant opening and reading raw log artifacts. Render a per-job
markdown table (category, pass/fail/skip, duration) to
$GITHUB_STEP_SUMMARY instead, visible directly on the run's Summary
page for each of the 13 matrix combinations: unit tests, Qt GUI tests,
util tests, functional tests (or debugscripts on Debug builds), and
benchmark (or its skip reason).

Existing fail-fast behavior (exit 1 on the first broken phase) is
preserved - each phase's if/else now also calls record_phase/
record_skipped before deciding whether to exit, so the summary reflects
however far the job got even on failure, not just full passes.

This is intentionally just the pass/fail/timing table for now; the
benchmark_results/data history-cache (already present but unpopulated)
and a trend graph per category are follow-up work.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Append each scheduled run's per-benchmark medians to a cached CSV and
render a log-scale trend chart (one line per benchmark) into the job
summary via a new contrib/devtools/plot_benchmark_history.py. Also pin
history collection to one canonical config per platform instead of
matching on platform+arch, since several matrix configs share the same
platform+arch and would otherwise race on the same cache key/file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@azuchi

azuchi commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Two things I'd like to see folded in before this lands. Both are small; R2 in particular is directly on-topic for this PR's title.

R1 — Move the benchmark-history opt-in from three literal matrix.config.name comparisons to a matrix field

The two canonical config names are hardcoded into daily-test.yml at:

  • :202Restore Benchmark History cache step's if:
  • :445Save Benchmark History cache step's if:
  • :778 — the in-line shell guard inside "Build and run daily tests" that decides whether to append to history.csv and render the chart

Renaming either config touches all three sites, and the failure mode of missing one is subtle (e.g. cache keeps persisting while the chart renders against stale data, or vice versa). Please add a matrix field:

matrix:
  config:
    - name: L-X86-Rel-GUI+USDT+Wallet
      ...
      collect_benchmark_history: true
    - name: M-ARM-Rel-GUI+Wallet
      ...
      collect_benchmark_history: true
    # others: field omitted → falsy

then all three if:s read matrix.config.collect_benchmark_history == true (and the shell guard becomes if [ "${{ matrix.config.collect_benchmark_history }}" = "true" ]; then). Adding/removing/renaming a canonical config becomes a one-field change.

R2 — Make daily-test.yml's concurrency group per-ref, matching smoke-test.yml

Current daily-test.yml:24-26:

concurrency:
  group: ${{ github.workflow }}-daily
  cancel-in-progress: true

The group name doesn't include github.ref, so every run of Cmake Daily Test — across every branch, PR, and schedule tick — competes for a single global slot. This is what caused this PR's daily run to cancel PR#439's mid-compile last week, and it will keep doing that whenever two PRs land daily runs within an hour of each other.

smoke-test.yml:16-18 already uses the correct pattern:

concurrency:
  group: ${{ github.workflow }}-${{ github.ref }}
  cancel-in-progress: true

Applying the same one-line change to daily-test.yml isolates runs per-branch, so a push to PR X only cancels PR X's own in-flight daily run (which is the actual intent of cancel-in-progress). This is squarely inside this PR's stated scope ("Daily smoke benchmark ci") and a one-line change, so I'd rather see it here than as a follow-up.

Other than these two, everything else in the current revision looks good — the lint fix works, smoke parallelism is confirmed by the green matrix, and the single L-X86-Deb-GUI+USDT+Wallet red on the latest run is feature_tokencreation.py (7-second functional-test failure, passes on L-ARM-Deb), which this PR's diff can't have caused. A re-run should clear it.

…ed, fix bugs in core dump creation steps and enable writing the backtrace to job summary
@Naviabheeman

Copy link
Copy Markdown
Contributor Author

Apart from adding the review recommendations, the last commit also fixes the core dump creation and debugging steps.

@azuchi
azuchi merged commit d1c88c4 into chaintope:master Jul 14, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants