Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ jobs:
- name: run conformance tests
# TODO: Debug stdin/stdout issues on Windows
if: ${{ !startsWith(matrix.os, 'windows-') }}
run: uv run pytest -rfEP ${{ matrix.coverage == 'cov' && '--cov=connectrpc --cov-report=xml' || '' }}
run: uv run pytest ${{ matrix.coverage == 'cov' && '--cov=connectrpc --cov-report=xml' || '' }}
working-directory: conformance

- name: run tests with minimal dependencies
Expand Down
2 changes: 1 addition & 1 deletion conformance/test/_util.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio
import sys

VERSION_CONFORMANCE = "v1.0.4"
VERSION_CONFORMANCE = "0bed9ca203aeda4e344edc442a4b2ede91726db5"
Copy link
Member

Choose a reason for hiding this comment

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

want to create an issue over in https://github.com/connectrpc/conformance/issues to tag a new version for connectrpc/conformance#1047? cc @jhump @doriable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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



async def create_standard_streams():
Expand Down
54 changes: 53 additions & 1 deletion conformance/test/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,50 @@ async def serve_hypercorn(
await proc.wait()


async def serve_pyvoy(
request: ServerCompatRequest,
mode: Literal["sync", "async"],
certfile: str | None,
keyfile: str | None,
cafile: str | None,
port_future: asyncio.Future[int],
):
args = ["--port=0"]
if certfile:
args.append(f"--tls-cert={certfile}")
if keyfile:
args.append(f"--tls-key={keyfile}")
if cafile:
args.append(f"--tls-ca-cert={cafile}")

if mode == "sync":
args.append("--interface=wsgi")
args.append("server:wsgi_app")
else:
args.append("server:asgi_app")

proc = await asyncio.create_subprocess_exec(
"pyvoy",
*args,
stderr=asyncio.subprocess.STDOUT,
stdout=asyncio.subprocess.PIPE,
env=_server_env(request),
)
stdout = proc.stdout
assert stdout is not None
stdout = _tee_to_stderr(stdout)
try:
async for line in stdout:
if b"listening on" in line:
port = int(line.strip().split(b"127.0.0.1:")[1])
port_future.set_result(port)
break
await _consume_log(stdout)
except asyncio.CancelledError:
proc.terminate()
await proc.wait()


async def serve_uvicorn(
request: ServerCompatRequest,
certfile: str | None,
Expand Down Expand Up @@ -655,7 +699,7 @@ def _find_free_port():
return s.getsockname()[1]


Server = Literal["daphne", "granian", "gunicorn", "hypercorn", "uvicorn"]
Server = Literal["daphne", "granian", "gunicorn", "hypercorn", "pyvoy", "uvicorn"]


class Args(argparse.Namespace):
Expand Down Expand Up @@ -728,14 +772,22 @@ async def main() -> None:
request, args.mode, certfile, keyfile, cafile, port_future
)
)
case "pyvoy":
serve_task = asyncio.create_task(
serve_pyvoy(
request, args.mode, certfile, keyfile, cafile, port_future
)
)
case "uvicorn":
if args.mode == "sync":
msg = "uvicorn does not support sync mode"
raise ValueError(msg)
serve_task = asyncio.create_task(
serve_uvicorn(request, certfile, keyfile, cafile, port_future)
)

asyncio.get_event_loop().add_signal_handler(signal.SIGTERM, serve_task.cancel)

port = await port_future
response = ServerCompatResponse()
response.host = "127.0.0.1"
Expand Down
37 changes: 11 additions & 26 deletions conformance/test/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def macos_raise_ulimit():

# There is a relatively low time limit for the server to respond with a resource error
# for this test. In resource limited environments such as CI, it doesn't seem to be enough,
# notably it is the first request that will take the longest to process as it also sets up
# notably it is the first message that will take the longest to process as it also sets up
# the request. We can consider raising this delay in the runner to see if it helps.
#
# https://github.com/connectrpc/conformance/blob/main/internal/app/connectconformance/testsuites/data/server_message_size.yaml#L46
Expand All @@ -37,24 +37,20 @@ def macos_raise_ulimit():
]


@pytest.mark.parametrize("server", ["granian", "gunicorn", "hypercorn"])
@pytest.mark.parametrize("server", ["gunicorn", "pyvoy"])
def test_server_sync(server: str) -> None:
if server == "pyvoy" and sys.platform == "win32":
pytest.skip("pyvoy not supported on Windows")

args = maybe_patch_args_with_debug(
[sys.executable, _server_py_path, "--mode", "sync", "--server", server]
)
opts = [
# While Hypercorn and Granian supports HTTP/2 and WSGI, they both have simple wrappers
# that reads the entire request body before running the application, which does not work for
# full duplex. There are no other popular WSGI servers that support HTTP/2, so in practice
# it cannot be supported. It is possible in theory following hyper-h2's example code in
# https://python-hyper.org/projects/hyper-h2/en/stable/wsgi-example.html though.
# TODO: Enable full-duplex in pyvoy
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This worked with some initial testing but I need to validate it some more

"--skip",
"**/bidi-stream/full-duplex/**",
]
match server:
case "granian" | "hypercorn":
# granian and hypercorn seem to have issues with concurrency
opts += ["--parallel", "1"]
case "gunicorn":
# gunicorn doesn't support HTTP/2
opts = ["--skip", "**/HTTPVersion:2/**"]
Expand All @@ -78,17 +74,14 @@ def test_server_sync(server: str) -> None:
check=False,
)
if result.returncode != 0:
if server == "granian":
# Even with low parallelism, some tests are flaky. We'll need to investigate further.
print( # noqa: T201
f"Granian server tests failed, see output below, not treating as failure:\n{result.stdout}\n{result.stderr}"
)
return
pytest.fail(f"\n{result.stdout}\n{result.stderr}")


@pytest.mark.parametrize("server", ["daphne", "granian", "hypercorn", "uvicorn"])
@pytest.mark.parametrize("server", ["daphne", "pyvoy", "uvicorn"])
def test_server_async(server: str) -> None:
if server == "pyvoy" and sys.platform == "win32":
pytest.skip("pyvoy not supported on Windows")

args = maybe_patch_args_with_debug(
[sys.executable, _server_py_path, "--mode", "async", "--server", server]
)
Expand All @@ -104,9 +97,6 @@ def test_server_async(server: str) -> None:
"--skip",
"**/full-duplex/**",
]
case "granian" | "hypercorn":
# granian and hypercorn seem to have issues with concurrency
opts = ["--parallel", "1"]
case "uvicorn":
# uvicorn doesn't support HTTP/2
opts = ["--skip", "**/HTTPVersion:2/**"]
Expand All @@ -115,6 +105,7 @@ def test_server_async(server: str) -> None:
"go",
"run",
f"connectrpc.com/conformance/cmd/connectconformance@{VERSION_CONFORMANCE}",
"-v",
"--conf",
_config_path,
"--mode",
Expand All @@ -129,10 +120,4 @@ def test_server_async(server: str) -> None:
check=False,
)
if result.returncode != 0:
if server == "granian":
# Even with low parallelism, some tests are flaky. We'll need to investigate further.
print( # noqa: T201
f"Granian server tests failed, see output below, not treating as failure:\n{result.stdout}\n{result.stderr}"
)
return
pytest.fail(f"\n{result.stdout}\n{result.stderr}")
5 changes: 4 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dev = [
"daphne==4.2.1",
"httpx[http2]==0.28.1",
"hypercorn==0.17.3",
"granian==2.5.5",
"granian==2.5.7",
"gunicorn==23.0.0",
"just-bin==1.42.4; sys_platform != 'win32'",
"mkdocs==1.6.1",
Expand All @@ -52,6 +52,8 @@ dev = [
"pytest==8.4.2",
"pytest-asyncio==1.2.0",
"pytest-cov==7.0.0",
"pytest-timeout==2.4.0",
"pyvoy==0.1.1; sys_platform != 'win32'",
"ruff~=0.13.2",
"uvicorn==0.37.0",
# Needed to enable HTTP/2 in daphne
Expand All @@ -72,6 +74,7 @@ module-name = "connectrpc"

[tool.pytest.ini_options]
testpaths = ["test"]
timeout = 1800 # 30 min

[tool.ruff.format]
skip-magic-trailing-comma = true
Expand Down
Loading