Skip to content

fix(test): align testRestartPolicy with the runtime's exit() contract - #249

Merged
Meldiron merged 1 commit into
mainfrom
fix/restart-policy-exit-contract
Aug 19, 2026
Merged

fix(test): align testRestartPolicy with the runtime's exit() contract#249
Meldiron merged 1 commit into
mainfrom
fix/restart-policy-exit-contract

Conversation

@Meldiron

Copy link
Copy Markdown
Contributor

Problem

main is currently red. testRestartPolicy fails deterministically:

1) Tests\E2E\ExecutorTest::testRestartPolicy
Failed asserting that 200 matches expected 500.

Nothing in this repo changed. openruntimes/php:v5-8.1 was re-pushed to Docker Hub on 2026-08-19 at 08:47 UTC, and .env pins mutable runtime tags, so CI picked up the rebuild.

Cause

The php runtime source in open-runtimes/runtimes is untouched — the rebuild pulled in a newer Swoole, where a function calling exit() raises Swoole\ExitException instead of tearing down the worker mid-response. The runtime now catches it and reports a failed execution. Probing the executor directly:

statusCode: 500
errors: Uncaught Swoole\ExitException: swoole exit in /usr/local/server/src/function/index.php:4

So exit(1) is now a clean failed execution — executor HTTP 200 with body.statusCode 500 — where it previously dropped the connection and surfaced an executor-level 500.

RestartCount=1 confirms the server process still dies afterwards, so the restart policy itself is unaffected and the assertSame(3, $occurances) assertion still holds. Only the two HTTP status expectations were stale.

Verification

  • Reproduced locally by pulling the new image; byte-identical to CI.
  • Confirmed present on clean origin/main, so it is independent of any open PR.
  • Full e2e suite on this branch: 39 tests, testRestartPolicy green, including everything past test 16 that CI never reached (stopOnFailure was aborting earlier).
  • format:check, analyze, refactor:check all pass.

Note for reviewers

Two things worth a maintainer's judgment, both out of scope here:

  1. Please confirm the new exit() semantics are intended before we codify them in a test. Graceful 500-with-error reads like an improvement over a dropped connection, but it arrived via a base-image rebuild rather than a deliberate change upstream.
  2. .env pins mutable tags (openruntimes/php:v5-8.1), so CI inherits upstream rebuilds like this one. Pinning digests would stop this class of surprise.

Also related: #248 was failing on testBuildKeysFlutter for a separate reason (the maintenance sweep reaping in-flight builds). That is fixed on that branch; this test was simply the next failure stopOnFailure had been hiding.

🤖 Generated with Claude Code

openruntimes/php:v5-8.1 was rebuilt on 2026-08-19 and picked up a newer
Swoole, where a function calling exit() raises Swoole\ExitException instead
of tearing the worker down mid-response. The runtime now catches it and
reports a failed execution, so the executor call succeeds with HTTP 200 and
carries statusCode 500, where it previously dropped the connection and
surfaced an executor-level 500.

The server process still exits afterwards, so the restart policy itself is
unaffected and the "3 server starts" assertion still holds. Only the two
HTTP status expectations were stale.

main has been red since that image was pushed, independently of any change
in this repo. Note that .env pins mutable runtime tags, so CI inherits
upstream rebuilds like this one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown

Greptile Summary

Updates testRestartPolicy to reflect the runtime execution API contract.

  • Expects executor HTTP status 200 for failed function executions.
  • Verifies the function failure through body.statusCode 500.
  • Retains the restart-count assertion that validates the restart policy.

Confidence Score: 5/5

The PR appears safe to merge, with the test now matching the established failed-execution response contract.

The execution endpoint returns HTTP 200 while carrying function failures in body.statusCode, and the unchanged restart-count assertion continues to verify that the server process restarts.

Important Files Changed

Filename Overview
tests/e2e/ExecutorTest.php The revised assertions match the execution endpoint’s HTTP/body status separation while preserving the test’s restart-policy verification.

Reviews (1): Last reviewed commit: "fix(test): align testRestartPolicy with ..." | Re-trigger Greptile

@Meldiron
Meldiron merged commit 0c3a65a into main Aug 19, 2026
7 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.

3 participants