Skip to content

Bound uplc-evaluator integration test teardown and run it nightly#7817

Open
Unisay wants to merge 4 commits into
masterfrom
yura/issue-2257-uplc-evaluator-ci-timeout
Open

Bound uplc-evaluator integration test teardown and run it nightly#7817
Unisay wants to merge 4 commits into
masterfrom
yura/issue-2257-uplc-evaluator-ci-timeout

Conversation

@Unisay

@Unisay Unisay commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

plutus-benchmark:test:uplc-evaluator-integration-tests intermittently consumed the full 2h Hydra wall clock and landed as TIMED_OUT, reddening x86_64-linux.required on unrelated PRs (IntersectMBO/plutus-private#2257). The root cause is in test teardown: Harness.stopService sent SIGTERM via terminateProcess and then called waitForProcess with no timeout, so any spawned uplc-evaluator that missed SIGTERM blocked teardown until the external CI cap. The suite starts 20+ services concurrently under tasty, so under CI load the odds of one wedging on shutdown are non-trivial.

This PR applies the three measures from the issue:

  • Bound the teardown (root cause). stopProcessBounded waits a grace period after SIGTERM, then escalates to SIGKILL via getPid + signalProcess, with the post-kill reap bounded too. A process that survives even SIGKILL is left for init to reap rather than blocking teardown. Cleanup now uses removePathForcibly. This pulls in unix, which is safe because the suite is already buildable: False on Windows via os-support.
  • Add a tasty per-test timeout safety net. The test tree is wrapped in adjustOption defaultTestTimeout (120s per test), applied only when no --timeout is given so it stays overridable. On its own it would not have fixed the hang, because when tasty times out a test the bracket cleanup still runs the teardown wait. So it is a safety net rather than the actual fix.
  • Take the suite off the per-PR required gate. The uplc-evaluator is a non-critical on-demand tool, so its integration tests need not gate every PR. The test executable is still built on every PR (the packages jobs stay in required), so a compile break still reddens PRs; only the test run moves to the nightly testsuite workflow.

A regression test drives a SIGTERM-ignoring process (trap '' TERM; exec sleep 600) and asserts teardown returns in bounded time with the process actually dead.

Verified locally: the regression test fails (times out) against the old unbounded teardown and passes in ~1.2s with the fix; the full suite is 29/29 both via cabal and via the exact nightly nix shell invocation; nix eval confirms the checks jobs are gone from required while the packages builds remain.

Closes https://github.com/IntersectMBO/plutus-private/issues/2257

Unisay added 3 commits June 15, 2026 16:14
Test teardown ran terminateProcess followed by an unbounded
waitForProcess, so a spawned service that missed SIGTERM hung the run
until the 2h CI cap (plutus-private#2257). Bound the wait and escalate to
SIGKILL, add a per-test tasty timeout as a safety net, and a regression
test that drives a SIGTERM-ignoring process.
The uplc-evaluator is a non-critical on-demand tool, so its integration
tests need not gate every PR. The test executable is still built on every
PR; the suite is run nightly instead. See plutus-private#2257.
@Unisay Unisay requested a review from Copilot June 15, 2026 14:37
@Unisay Unisay self-assigned this Jun 15, 2026
@Unisay Unisay requested a review from zeme-wana June 15, 2026 14:37
@Unisay Unisay marked this pull request as ready for review June 15, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the uplc-evaluator integration test suite against teardown hangs by bounding process shutdown, adds a per-test timeout safety net, and moves execution of the suite from the per-PR required gate to the nightly workflow (while still building the test executable on PRs).

Changes:

  • Implement bounded service shutdown in the test harness (SIGTERM → bounded wait → SIGKILL escalation → bounded reap) and make cleanup more robust.
  • Wrap the tasty tree with a default per-test timeout (only when the user hasn’t provided --timeout) and add a regression test covering SIGTERM-ignoring processes.
  • Adjust Nix/Hydra outputs and the nightly GitHub Actions workflow so the integration tests run nightly instead of gating every PR.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plutus-benchmark/uplc-evaluator/test/Spec.hs Adds default tasty timeout wrapper and a regression test validating bounded teardown behavior.
plutus-benchmark/uplc-evaluator/test/Harness.hs Introduces stopProcessBounded and switches teardown cleanup to removePathForcibly.
plutus-benchmark/plutus-benchmark.cabal Adds unix dependency for POSIX signaling used by bounded shutdown.
nix/outputs.nix Removes the integration test check from required Hydra jobs while still exposing/building the test derivation.
.github/workflows/nightly-testsuite.yml Runs uplc-evaluator integration tests in the nightly workflow.

Comment thread plutus-benchmark/uplc-evaluator/test/Harness.hs Outdated
- plutus-benchmark/uplc-evaluator/test/Harness.hs:126 — re-check getProcessExitCode before SIGKILL escalation (#7817 (comment))
@Unisay Unisay added the No Changelog Required Add this to skip the Changelog Check label Jun 16, 2026
@Unisay Unisay enabled auto-merge (squash) June 16, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No Changelog Required Add this to skip the Changelog Check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants