Skip to content

Run tests in parallel in weekly CI#442

Open
Naviabheeman wants to merge 1 commit into
chaintope:masterfrom
Naviabheeman:weeklyHeavyTestTimeoutFix
Open

Run tests in parallel in weekly CI#442
Naviabheeman wants to merge 1 commit into
chaintope:masterfrom
Naviabheeman:weeklyHeavyTestTimeoutFix

Conversation

@Naviabheeman

Copy link
Copy Markdown
Contributor

Last weekly CI job ended in timeout as Github action can run only for 6 hours. Change the job a little to improve parallelism:

  • Collapsed feature-pruning/feature-dbcrash into one heavy-tests job with strategy.matrix — build pipeline lives once, matrix-driven test_args/name differentiate the two.
  • Added step to Print full combined logs on failure: runs combine_logs.py directly over every test tmpdir and writes the full, untruncated output (all nodes + test framework log) to — works even for a Timeout result, which test_runner.py itself never surfaces to the console.
  • Retained the dead-timeout removal, chmod cleanup, ulimit fix, and dangling-process kill from the earlier split.

…with strategy.matrix — build pipeline lives once, matrix-driven test_args/name differentiate the two.

- Added step to Print full combined logs on failure: runs combine_logs.py directly over every test tmpdir and writes the full, untruncated output (all nodes + test framework log) to  — works even for a Timeout result, which test_runner.py itself never surfaces to the console.
- Retained the dead-timeout removal, chmod cleanup, ulimit fix, and dangling-process kill from the earlier split.
@azuchi

azuchi commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Matrix split shape is right. A few items worth resolving before merge:

Recommended (in-scope)

  1. if: failure() on the combined-logs step won't fire on the failure mode this PR targets. A job hitting the 6h runner ceiling terminates with conclusion: cancelled, not failure, so the log-dump step is skipped in exactly the case that motivated it. Use if: failure() || cancelled().

  2. $GITHUB_STEP_SUMMARY has a 1 MB per-step cap. combine_logs.py on a heavy test easily produces tens of MB and GitHub silently drops the excess, so the "full untruncated" claim doesn't hold in practice. Either tail -c 950000 before appending, or write the full log to a file and upload it as an artifact with the summary linking to it.

  3. concurrency.group: ${{ github.workflow }}-heavy (line 28) is a single global slot. Same shape as daily-test.yml's old group that PR Daily smoke benchmark ci #440 fixed to ${{ github.workflow }}-${{ github.ref }}. Weekly contention is rarer but a workflow_dispatch during a schedule run still cancels the earlier one unnecessarily. Apply the same one-line fix here for consistency.

Nit (fine as follow-up)

  1. Debug core dump (on failure) is the pre-Daily smoke benchmark ci #440 shape — single most-recent dump only, hardcoded tapyrusd binary, no summary output. PR Daily smoke benchmark ci #440 rewrote the daily version to loop over every core.*, resolve the crashing binary from file output, and emit bt full into $GITHUB_STEP_SUMMARY. Copy that block verbatim.

  2. HEAVY_SCRIPTS in test_runner.py:255 is now a second source of truth for "what runs weekly." Before: workflow ran --heavy, list lived in one place. After: the matrix drives it, and HEAVY_SCRIPTS is only kept alive by the completeness check at test_runner.py:663. Add a one-line comment on HEAVY_SCRIPTS saying "kept for the completeness check only; the weekly workflow drives what actually runs" so a future editor knows both places need updating together.

Also — the weekly workflow has no runs on this SHA (only schedule + workflow_dispatch triggers). A workflow_dispatch against this branch before merging would exercise the matrix split + combine_logs.py path for real.

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