Skip to content

[Tests] Stop php -S servers together with their forked workers - #658

Merged
chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
ruudk:fix/stop-php-server-workers
Nov 3, 2026
Merged

chr-hertel merged 1 commit into
modelcontextprotocol:mainfrom
ruudk:fix/stop-php-server-workers

Conversation

@ruudk

@ruudk ruudk commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Stopping a php -S process only ended the master, so its PHP_CLI_SERVER_WORKERS were reparented and kept the port and the inherited stdout/stderr open, which made make integration-tests | tail hang after the suite finished. Every test that starts a php -S server now stops it through ServerProcess::stop(), which ends the whole process tree.

@chr-hertel chr-hertel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @ruudk - good one, could you please streamline the other cases as well?

  • tests/Integration/HttpNegotiationTest.php:48
  • tests/Integration/StatelessClientTest.php:67
  • tests/Integration/StatelessLifecycleTest.php:58
  • tests/Interop/Server/Http/HttpInspectorSnapshotTestCase.php:73

Stopping the php -S process only ended the master. With PHP_CLI_SERVER_WORKERS the workers were reparented, kept their port and the inherited stdout/stderr, so `make integration-tests | tail` never saw EOF. Stop every php -S server the tests start through ServerProcess::stop(), which ends the whole process tree.
@ruudk
ruudk force-pushed the fix/stop-php-server-workers branch from 6117756 to f3b98c1 Compare November 3, 2026 13:39
@ruudk ruudk changed the title [Tests] Stop the forked php -S workers of the dual-era example tests [Tests] Stop php -S servers together with their forked workers Nov 3, 2026
@ruudk

ruudk commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Done!

@ruudk
ruudk requested a review from chr-hertel November 3, 2026 14:05

@chr-hertel chr-hertel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @ruudk!

@chr-hertel
chr-hertel merged commit b77af64 into modelcontextprotocol:main Nov 3, 2026
28 checks passed
chr-hertel added a commit to BrocksiNet/php-sdk that referenced this pull request Nov 4, 2026
…et/php-sdk into pr-618-end-a-listen-stream-that-can

* 'fix/listen-empty-filter' of https://github.com/BrocksiNet/php-sdk: (25 commits)
  [Server] Pass an explicit null to a nullable tool parameter with a default (modelcontextprotocol#642)
  [Capability] Check tool names against the spec's 128-character limit and character set (modelcontextprotocol#639)
  [Tests] Stop php -S servers together with their forked workers (modelcontextprotocol#658)
  chore(deps): bump astral-sh/setup-uv from 10.3.2 to 10.3.3 (modelcontextprotocol#659)
  [Server] Accept plain http on `*.localhost` and `127.0.0.0/8` hosts in OAuth URLs (modelcontextprotocol#620)
  [Capability] Map PHPDoc pseudo-types to the JSON Schema type of their base type (modelcontextprotocol#643)
  Raise minimum versions of `phpdocumentor/reflection-docblock` and `psr/container` (modelcontextprotocol#657)
  [Server] Use compact JSON in result formatters (modelcontextprotocol#647)
  [Server] Filter list completions in one pass (modelcontextprotocol#649)
  [Server] Fix result lists being sent as JSON objects (modelcontextprotocol#656)
  Avoid repeated array merges in prompt formatting (modelcontextprotocol#650)
  [Docs] Add AGENTS.md (modelcontextprotocol#634)
  Avoid repeated array merges in resource formatting (modelcontextprotocol#651)
  [Server] Fix `"0"` prefix offering all completions (modelcontextprotocol#655)
  Filter enum completions in one pass (modelcontextprotocol#652)
  Paginate registry references before mapping (modelcontextprotocol#653)
  Extract JWT scopes in one pass (modelcontextprotocol#654)
  [Server] Fix `Session::save()` wiping a session that the store failed to read (modelcontextprotocol#637)
  [Server] Answer a DELETE of an unknown or ended session with 404 (modelcontextprotocol#638)
  [Server] Add `Psr6SessionStore` and `Psr6NotificationBus` (modelcontextprotocol#635)
  ...
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