internetarchive / internetarchive/openlibrary

Documented local test command fails: LOCAL_DEV fallback route breaks 5 FastAPI route-registration tests

Open
#13,587 3 comments 0 reactions 0 assignees View on GitHub
Affects: Configuration Affects: Server Lead: @RayBB Module: Docker Needs: Response Needs: Triage Regression Theme: Development Theme: Testing Type: Bug
Dominant language
Python
Stars
6.7k
Forks
2k
Avg merge
2d 19h
Merged PRs (30d)
138

Description

### Problem

`docker compose run --rm home make test` — the test command from the [Quick Start](https://docs.openlibrary.org/developers/quick-start.html) — fails on a clean checkout of master with 5 failures, while CI is green on the same commit.

```
FAILED openlibrary/tests/fastapi/test_borrow.py::TestBorrowRoute::test_missing_slug_404s
FAILED openlibrary/tests/fastapi/test_cdn.py::TestIaJsCdn::test_various_invalid_filenames_return_404[../../etc/passwd]
FAILED openlibrary/tests/fastapi/test_cdn.py::TestIaJsCdn::test_various_invalid_filenames_return_404[]
FAILED openlibrary/tests/fastapi/test_hide_banner.py::TestHideBannerEndpoint::test_only_post_without_json_suffix_is_registered
FAILED openlibrary/tests/fastapi/test_qrcode.py::TestQRCodeEndpoint::test_only_get_is_registered
= 5 failed, 5956 passed, 41 skipped, 5 deselected, 1 xfailed in 58.78s =
make: *** [Makefile:104: test] Error 2
```

The cause is `LOCAL_DEV`. At `openlibrary/asgi_app.py:275` the catch-all proxy route is registered only in local dev:

```python
if get_ol_env().LOCAL_DEV or get_deployment_name() == "testing":
@app.api_route("/{path:path}", methods=["GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "HEAD"])
async def fallback(request: Request, path: str) -> Response:
```

Those 5 tests assert that a method or path is *not* registered — `test_only_get_is_registered` does `POST /qrcode` and expects 405. With the fallback present that request is captured and proxied to `web:8080` instead, which the test container can't resolve, so the assertion sees an `httpx.ConnectError` rather than a 405.

The `home` container sets `LOCAL_DEV=true`, so the documented local command runs these tests in the one environment where they cannot pass. CI never sets `LOCAL_DEV`, so it cannot catch this.

This is a recent regression rather than longstanding. The fallback landed 2026-08-26 in #13423; checking out its parent `2f3f64979` and running the same four files in the same container, with `LOCAL_DEV` left at the container default, gives `39 passed`. The same command on master gives the 5 failures above.

Toggling only that variable is enough to flip them:

```
$ docker compose run --rm --no-deps -T home python -m pytest -q
5 failed

$ docker compose run --rm --no-deps -T -e LOCAL_DEV=false home python -m pytest -q
39 passed
```

### Reproducing the bug

1. Clone master fresh (I was on `e5b6d22c8`), `make git`, `docker compose up -d`
2. `docker compose run --rm home make test`

* Expected behavior: the suite passes, as it does in CI on the same commit
* Actual behavior: 5 failures and `make` exits non-zero

Adding `-e LOCAL_DEV=false` to the same command makes all 5 pass.

### Context

- Browser (Chrome, Safari, Firefox, etc): n/a
- OS (Windows, Mac, etc): Windows 11 + WSL2, Ubuntu 26.04
- Logged in (Y/N): n/a
- Environment (prod, dev, local): local — Docker Engine 29.1.3, Compose 2.40.3, no Docker Desktop

### Breakdown

Implementation Details (for maintainers)

Two directions, and I didn't want to assume which you'd prefer. The `fastapi_client` fixture could build the app with `LOCAL_DEV` forced off, since these are unit tests of route registration rather than of the dev proxy — that keeps them meaningful in both environments. Alternatively the 5 tests could account for the fallback being present, though that makes them assert proxy behaviour rather than what they were written to check.

Worth noting either way that CI cannot regress-test this, since it never sets `LOCAL_DEV`.

#### Requirements Checklist
* [ ] `docker compose run --rm home make test` passes on a clean checkout
* [ ] the 5 route-registration tests stay meaningful with the dev fallback registered

#### Related files

* `openlibrary/asgi_app.py:275` — the `LOCAL_DEV`-gated catch-all `fallback` route
* `openlibrary/tests/fastapi/conftest.py:19` — `fastapi_client`, builds the real app via `create_app()`
* `openlibrary/tests/fastapi/` — `test_borrow.py`, `test_cdn.py`, `test_hide_banner.py`, `test_qrcode.py`

#### Stakeholders

* @RayBB (#13423, added the fallback route)


#### Instructions for Contributors

- Please [run these commands](https://docs.openlibrary.org/developers/tools/git.html#working-on-your-branch) to ensure your repository is up to date **before** [creating a new branch](https://docs.openlibrary.org/developers/tools/git.html#making-changes-and-creating-a-pull-request) to work on this issue and **each time after** pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.

Contributor guide

Open the contributing guide

Research direction

Start by reading openlibrary/asgi_app.py:275 and the fastapi_client fixture in openlibrary/tests/fastapi/conftest.py:19. Run the four named test files with the default LOCAL_DEV setting and with LOCAL_DEV=false, then inspect the route-registration tests. Done means docker compose run --rm home make test passes and the five tests remain meaningful when the local fallback is present.

Written by the indexing model from the issue text.

Assessment

Tech stack
docker, fastapi, python
Domain
api, backend, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.