[Draft] chore: test Hyperdrive#152
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds Hyperdrive integration tests for MySQL (pymysql) and PostgreSQL (pg8000) using the same two-layer test pattern as the bindings tests. I see the PR description mentions it's not ready yet, but here's early feedback on issues that should be fixed before merging.
Issues (highest severity first)
-
test_hyperdrive.pyimportsCOMPAT_DATESwhich does not exist inconftest.py— The shared conftest exportsCOMPAT_CONFIGS(a list ofCompatConfigobjects), notCOMPAT_DATES. The bindings test file (test_bindings.py) is the up-to-date reference. This will cause anImportErrorat test collection time. -
test_hyperdrive.pydoesn't callinject_compat_flags()— Thecompat_datefixture only passes a bare string, not aCompatConfig. Without injecting the extra compat flags (e.g.enable_python_external_sdk,python_workers_20260610), the tests will fail for all Python versions since each requires specific flags. -
test_hyperdrive.pyuv runcommand is missing--no-project— The bindings test usesuv run --no-project --with ...to avoiduvresolving against a parentpyproject.toml. Without--no-project,uv runmay pick up the wrong project context. -
worker.pyis missing the event loop save/restore aroundpytest.main()— The bindings worker saves and restores the running event loop (asyncio.events._get_running_loop()) aroundpytest.main()calls to work around a Pyodide WebLoop issue. The hyperdrive worker lacks this, which will cause subsequent requests to hang on a dead loop. -
AssertionErrortypo inworker.py— Line 100 usesexcinfo.errisinstance(AssertionError)which referencesAssertionError(the correct Python name), but note this same pattern exists in the bindings worker — just flagging for awareness that this is a deliberatepytestinternal API call, not a typo. (No action needed.)
Items 1-4 are all patterns already solved in test_bindings.py and its worker — aligning with that file should fix everything.
| because the database setup is non-trivial. In CI, databases are provided via Docker services | ||
| in the hyperdrive-tests.yml workflow. | ||
|
|
||
| To run locally: |
There was a problem hiding this comment.
Import will fail: conftest.py does not export COMPAT_DATES. It exports COMPAT_CONFIGS (a list of CompatConfig objects). The fixture and imports should be updated to match the pattern in test_bindings.py.
| To run locally: | |
| from conftest import ( | |
| COMPAT_CONFIGS, | |
| CompatConfig, | |
| inject_compat_flags, | |
| replace_compat_date, | |
| ) |
| f"stdout: {stdout}" | ||
| ) | ||
| try: | ||
| resp = requests.get(f"{base_url}/health", timeout=2) |
There was a problem hiding this comment.
The fixture should use COMPAT_CONFIGS and yield a CompatConfig object (not a bare string), matching the bindings test pattern. This also enables inject_compat_flags() support.
| f"stdout: {stdout}" | |
| ) | |
| try: | |
| resp = requests.get(f"{base_url}/health", timeout=2) | |
| @pytest.fixture( | |
| scope="module", | |
| params=COMPAT_CONFIGS, | |
| ids=[c.python_version for c in COMPAT_CONFIGS], | |
| ) | |
| def compat_config(request: pytest.FixtureRequest) -> CompatConfig: | |
| return request.param |
| except (requests.ConnectionError, requests.Timeout): | ||
| time.sleep(DEV_POLL_INTERVAL) | ||
|
|
||
| process.kill() | ||
| process.wait() | ||
| pytest.fail(f"pywrangler dev did not become ready within {DEV_STARTUP_TIMEOUT}s") | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module", params=COMPAT_DATES) | ||
| def compat_date(request: pytest.FixtureRequest) -> str: | ||
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(scope="module") | ||
| def dev_server( |
There was a problem hiding this comment.
Two issues here:
- The fixture parameter should be
compat_config: CompatConfiginstead ofcompat_date: str. inject_compat_flags()must be called afterreplace_compat_date()— without it, tests will fail for Python versions that require extra compat flags.
| except (requests.ConnectionError, requests.Timeout): | |
| time.sleep(DEV_POLL_INTERVAL) | |
| process.kill() | |
| process.wait() | |
| pytest.fail(f"pywrangler dev did not become ready within {DEV_STARTUP_TIMEOUT}s") | |
| @pytest.fixture(scope="module", params=COMPAT_DATES) | |
| def compat_date(request: pytest.FixtureRequest) -> str: | |
| return request.param | |
| @pytest.fixture(scope="module") | |
| def dev_server( | |
| def dev_server( | |
| tmp_path_factory: pytest.TempPathFactory, compat_config: CompatConfig | |
| ) -> Generator[str]: | |
| tmp_path = tmp_path_factory.mktemp("hyperdrive_test") | |
| target = tmp_path / "hyperdrive-test" | |
| shutil.copytree(HYPERDRIVE_TEST_DIR, target) | |
| env = os.environ | {"_PYODIDE_EXTRA_MOUNTS": str(tmp_path)} | |
| wrangler_jsonc = target / "wrangler.jsonc" | |
| replace_compat_date(wrangler_jsonc, compat_config.compat_date) | |
| inject_compat_flags(wrangler_jsonc, compat_config.extra_compat_flags) | |
| pywrangler_cmd = ["uv", "run", "--no-project", "--with", WORKERS_PY, "pywrangler"] | |
| subprocess.run( |
| tmp_path_factory: pytest.TempPathFactory, compat_date: str | ||
| ) -> Generator[str]: | ||
| tmp_path = tmp_path_factory.mktemp("hyperdrive_test") | ||
| target = tmp_path / "hyperdrive-test" | ||
| shutil.copytree(HYPERDRIVE_TEST_DIR, target) | ||
| env = os.environ | {"_PYODIDE_EXTRA_MOUNTS": str(tmp_path)} |
There was a problem hiding this comment.
Should use the pywrangler_cmd variable (with --no-project) like in test_bindings.py, to avoid uv resolving against a parent pyproject.toml.
| tmp_path_factory: pytest.TempPathFactory, compat_date: str | |
| ) -> Generator[str]: | |
| tmp_path = tmp_path_factory.mktemp("hyperdrive_test") | |
| target = tmp_path / "hyperdrive-test" | |
| shutil.copytree(HYPERDRIVE_TEST_DIR, target) | |
| env = os.environ | {"_PYODIDE_EXTRA_MOUNTS": str(tmp_path)} | |
| subprocess.run( | |
| [*pywrangler_cmd, "sync"], | |
| cwd=target, | |
| check=True, | |
| env=env, | |
| ) |
| ["uv", "run", "--with", WORKERS_PY, "pywrangler", "sync"], | ||
| cwd=target, | ||
| check=True, | ||
| env=env, | ||
| ) | ||
|
|
||
| shutil.copytree(WORKERS_RUNTIME_SDK, target / "python_modules", dirs_exist_ok=True) | ||
|
|
||
| port: int = _get_free_port() | ||
| base_url: str = f"http://127.0.0.1:{port}" | ||
|
|
||
| process = subprocess.Popen( | ||
| [ | ||
| "uv", | ||
| "run", |
There was a problem hiding this comment.
Same issue — should use the shared pywrangler_cmd list with --no-project.
| ["uv", "run", "--with", WORKERS_PY, "pywrangler", "sync"], | |
| cwd=target, | |
| check=True, | |
| env=env, | |
| ) | |
| shutil.copytree(WORKERS_RUNTIME_SDK, target / "python_modules", dirs_exist_ok=True) | |
| port: int = _get_free_port() | |
| base_url: str = f"http://127.0.0.1:{port}" | |
| process = subprocess.Popen( | |
| [ | |
| "uv", | |
| "run", | |
| process = subprocess.Popen( | |
| [ | |
| *pywrangler_cmd, | |
| "dev", | |
| "--port", | |
| str(port), | |
| "--persist-to", | |
| str(tmp_path / "state"), | |
| ], | |
| cwd=target, | |
| stdout=subprocess.PIPE, | |
| stderr=subprocess.STDOUT, | |
| text=True, | |
| env=env, | |
| ) |
| if importlib.util.find_spec(module) is None: | ||
| return Response.json( | ||
| {"error": f"Unknown suite '{suite_name}' (no module '{module}')"}, | ||
| status=404, | ||
| ) | ||
|
|
||
| collector = ResultCollector() | ||
| pytest.main( | ||
| ["--pyargs", module, "-p", "no:cacheprovider"], | ||
| plugins=[collector, EnvPlugin(self.env)], | ||
| ) | ||
| return Response.json(collector.results) |
There was a problem hiding this comment.
Missing the event loop save/restore workaround that bindings-test/src/worker.py has. Without this, after the first pytest.main() call, the running loop will point at an abandoned WebLoop, causing subsequent requests to hang.
| if importlib.util.find_spec(module) is None: | |
| return Response.json( | |
| {"error": f"Unknown suite '{suite_name}' (no module '{module}')"}, | |
| status=404, | |
| ) | |
| collector = ResultCollector() | |
| pytest.main( | |
| ["--pyargs", module, "-p", "no:cacheprovider"], | |
| plugins=[collector, EnvPlugin(self.env)], | |
| ) | |
| return Response.json(collector.results) | |
| def _run_suite(self, suite_name): | |
| module = f"test_{suite_name}" | |
| if importlib.util.find_spec(module) is None: | |
| return Response.json( | |
| {"error": f"Unknown suite '{suite_name}' (no module '{module}')"}, | |
| status=404, | |
| ) | |
| collector = ResultCollector() | |
| saved_loop = asyncio.events._get_running_loop() | |
| try: | |
| pytest.main( | |
| ["--pyargs", module, "-p", "no:cacheprovider"], | |
| plugins=[collector, EnvPlugin(self.env)], | |
| ) | |
| finally: | |
| asyncio.events._set_running_loop(saved_loop) | |
| return Response.json(collector.results) |
|
I'm Bonk, and I've done a quick review of your PR. This PR adds Hyperdrive integration tests for MySQL and PostgreSQL. I posted a batch review on PR #152 with 4 actionable issues (all with concrete suggestion comments):
All four are already solved in |
not ready to be reviewed yet. It seems there is an issue with tls connection