Skip to content

fix: make the project build, test, and document cleanly on macOS - #34

Open
Rutasd wants to merge 4 commits into
evaluation-context-protocol:mainfrom
Rutasd:fix/macos-support
Open

Rutasd wants to merge 4 commits into
evaluation-context-protocol:mainfrom
Rutasd:fix/macos-support

Conversation

@Rutasd

@Rutasd Rutasd commented Aug 30, 2026

Copy link
Copy Markdown

Problem

The project does not build, test, or document cleanly on macOS. Adding macOS to CI turned up a real SDK bug behind the two HTTP integration tests, so this ended up larger than a docs patch.

1. A test fails on a clean checkout. test_run_passes_explicit_timeout_to_runner fails on macOS, 1 of 78:

AssertionError: expected call not found.
Expected: mock(..., manifest_path='/var/folders/l8/.../tmpcis1j9es.yaml')
  Actual: mock(..., manifest_path='/private/var/folders/l8/.../tmpcis1j9es.yaml')

--manifest is declared with resolve_path=True (runtime/python/src/ecp_runtime/cli.py:62), so the CLI hands the runner a resolved path. The test asserted against the raw path from tempfile, and on macOS the temp directory sits behind the /var to /private/var symlink. Test bug, not a runtime bug.

2. serve_http can take 35 seconds to accept connections. http.server.HTTPServer.server_bind calls socket.getfqdn() on the bound host to populate server_name. That is a reverse DNS lookup, and where the resolver is slow or has no PTR record it blocks. Measured on a GitHub macOS runner:

getfqdn('127.0.0.1') took 35.01s -> 'localhost'
ThreadingHTTPServer(...) took 0.00s on port 49393

TCPServer.__init__ binds the socket before that lookup and calls listen() only after it. For the whole 35 seconds the port is bound but refusing connections, and serve_http has not yet printed its listening line, so a client sees connection refused rather than a server that is slow to start. This is what fails test_async_python_demo_conforms_over_http and test_streamable_http_demo_manifest_passes on macOS, and it would hit any user on a network with slow or absent reverse DNS, not only CI.

3. CI could not catch either. tests.yml and ci.yml are both runs-on: ubuntu-latest with no OS axis.

4. The docs are PowerShell-only. README.md, CONTRIBUTING.md, and docs/quickstart.md each give exactly one setup path, using py -m venv and .\.venv\Scripts\Activate.ps1. There are no macOS or Linux instructions anywhere. Since CI runs Ubuntu only, the documented path was never tested and the tested path was never documented. In CONTRIBUTING.md the PYTHONPATH examples are wrong on macOS twice over: the assignment syntax is PowerShell, and the multi-path examples use ; where POSIX shells need :.

Changes

sdk/python/src/ecp/server.py_ECPThreadingHTTPServer overrides server_bind to skip the getfqdn lookup and use the host as given. server_name only ever reaches CGI-style headers. Covered by a new test in sdk/python/tests/test_server.py that fails if the lookup returns.

runtime/python/tests/test_cli.py — compare against the resolved path. A no-op where the temp directory is already real.

runtime/python/tests/test_example_integrations.py — when a server never comes up, report the server's exit code, stdout, and stderr rather than only the port number, and fail immediately if the process has already exited. The original failure said only Timed out waiting for 127.0.0.1:49177, which is what made the cause above hard to see.

.github/workflows/tests.yml — add one macos-latest job, on a single interpreter rather than the full five-version matrix, since macOS runners bill at a multiple of Linux and the differences that bite here are filesystem and resolver behaviour rather than interpreter version. The matrix goes from 5 jobs to 6. Also fail-fast: false, so one platform failing does not cancel the others.

README.md, docs/quickstart.md, CONTRIBUTING.md — add macOS and Linux commands next to the existing Windows ones rather than replacing them, and convert the CONTRIBUTING.md PYTHONPATH examples to POSIX form with a note on the PowerShell translation.

I did not use tabbed-content syntax, because mkdocs.yml enables only fenced_code and tables and it would render as literal text.

One extra line in CONTRIBUTING.md: python -m pip install --upgrade pip before the editable installs. Both packages build with hatchling, and the pip shipped with the system Python on macOS (21.2.4) predates PEP 660, so following Local Setup verbatim fails with Directory cannot be installed in editable mode.

Verification

Full matrix green on a fork run of this exact branch, including macos-latest: 5 Ubuntu jobs (3.9 through 3.13) and the new macOS job.

Locally on macOS 15 (arm64), from a clean virtualenv following the updated CONTRIBUTING.md verbatim:

Check Python 3.9.6 Python 3.12.13
runtime/python/tests 78 passed 78 passed
sdk/python/tests 37 passed 38 passed
ruff check (as ci.yml runs it) passed
mkdocs build --strict built, no warnings
Flagship demo smoke test validates and runs validates and runs

Both test changes are load-bearing, confirmed by reverting each alone: test_cli.py goes 13 passed to 1 failure, and the new bind test fails when _build_http_server returns a plain ThreadingHTTPServer.

Notes

Two things found while verifying this that I did not touch, happy to file separately:

  • Adding windows-latest to the matrix has a good argument, given the documented setup path is PowerShell and nothing tests it. Left out because I cannot verify it passes.
  • ecp run --json writes 164 bytes of LiteLLM banner to stdout ahead of the JSON when a judge grader errors without OPENAI_API_KEY, so the payload docs/ci.md recommends for CI is not parseable in that case. The JSON body is valid once the banner is stripped. Looks related to Move litellm to an optional extra #29.

Rutasd added 4 commits August 30, 2026 09:44
--manifest is declared with resolve_path=True, so the CLI passes the runner
a fully resolved path. The test asserted against the raw path returned by
tempfile. On macOS the temp directory sits behind the /var -> /private/var
symlink, so the two differ and test_run_passes_explicit_timeout_to_runner
fails on a clean checkout.

Compare against the resolved path instead. This is a no-op on platforms
where the temp directory is already a real path.
The suite ran on ubuntu-latest only, so the macOS path failure fixed in the
previous commit could not be caught upstream.

Add one macos-latest job on a single interpreter rather than the full
five-version matrix: macOS runners bill at a multiple of Linux, and the
differences that bite here are filesystem behaviour rather than interpreter
version. Set fail-fast: false so one platform failing does not cancel the
others, which is the case this matrix exists to report.
README.md, CONTRIBUTING.md, and docs/quickstart.md each documented exactly
one setup path, in PowerShell. CI runs Ubuntu only, so the documented path
was never tested and the tested path was never documented.

Add the macOS and Linux equivalents next to the existing Windows commands,
and convert the PYTHONPATH examples in CONTRIBUTING.md to the POSIX form
with a note on the PowerShell translation. The path separator differs there
too, so the multi-path examples were wrong on macOS twice over.

Also note the pip upgrade needed before the editable installs. Both packages
build with hatchling, and the pip that ships with the system Python on macOS
predates PEP 660 support, so 'pip install -e runtime/python' fails with
'Directory cannot be installed in editable mode'.
…HTTP server

http.server.HTTPServer.server_bind calls socket.getfqdn() on the bound host
to populate server_name. That is a reverse DNS lookup. Where the resolver is
slow or has no PTR record for the address it blocks; measured at 35.01s for
127.0.0.1 on a GitHub macOS runner.

TCPServer.__init__ binds the socket before that lookup and calls listen()
only after it, so for the whole window the port is bound but refuses
connections and serve_http has not yet printed its listening line. A caller
sees connection refused rather than a server that is merely slow to start.
This is what made the two HTTP example integration tests fail on macOS.

Override server_bind to skip the lookup and use the host as given.
server_name only ever reaches CGI-style headers.

Also make the integration tests report why a server never came up: on
timeout they now surface the server's exit code, stdout, and stderr instead
of only the port number, and they fail immediately if the process has
already exited.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants