fix: make pytest suite hermetic against local dev-machine state#647
Merged
Conversation
cesschneider
requested review from
ANarayan,
jonsaadfalcon and
robbym-dev
as code owners
July 17, 2026 02:51
Two sources of non-deterministic local failures in `pytest tests/`: - The CLI's PyPI update-check nag writes its banner to stderr, which Click's CliRunner merges into result.output. It self-disables under CI=true, but a dev running tests locally with network access and a stale version-check cache gets the banner appended to JSON/CSV CLI output, breaking every test that parses it. Disable it unconditionally for the whole suite via an autouse fixture in tests/conftest.py. - tests/tools/storage/test_dense.py's Ollama skip-guard only checked TCP connectivity, not whether the `nomic-embed-text` embed model was pulled. On a machine running Ollama for chat models but without that specific model, the tests errored instead of skipping as documented. _ollama_up() now checks /api/tags for the model. Also required rebuilding the openjarvis_rust PyO3 extension (missing — most core modules hard-require it) and re-syncing the dev/server extras that a bare `uv sync`/`uv run` had silently dropped relative to the desktop-native dependency group.
cesschneider
force-pushed
the
fix/pytest-failures
branch
from
July 17, 2026 04:36
c2dc63d to
18a8f53
Compare
…d endpoint into backend - OLLAMA_HOST is documented as a full URL (http://host:11434); the probe built http://<url>:<port>/api/tags from it. Normalize full-URL, host:port, and bare-host forms into one base URL. - DenseMemory() built its embedder with the hard-coded localhost default, so a remote OLLAMA_HOST could pass the probe and then have every test hit the wrong server. Ollama-backed tests now construct the backend against the probed endpoint. - Drop OLLAMA_PORT (test-only knob the runtime never honored). - Unit tests for the probe: server down, model missing, tagged model, configured URL.
ElliotSlusky
approved these changes
Jul 17, 2026
ElliotSlusky
left a comment
ElliotSlusky
left a comment
Collaborator
There was a problem hiding this comment.
Both fixes verified: the update-check suppression is correct (the dedicated version-check tests manage the env var themselves and all 33 pass), and the model-aware Ollama probe reproduces correctly on a machine running Ollama without nomic-embed-text (13 passed / 9 skipped instead of errors). Pushed a follow-up: the probe now normalizes OLLAMA_HOST's documented full-URL form (it previously built http://:/api/tags), and the Ollama-backed tests construct DenseMemory against the same probed endpoint instead of the embedder's hard-coded localhost default, with unit tests for the probe itself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Running
uv run pytest tests/ -von a fresh dev machine (after building the mandatoryopenjarvis_rustextension) left 13 failures/6 errors that don't reproduce in CI, because CI's environment happens to sidestep two hermeticity gaps in the test suite:check_for_updates()writes its "new version available" banner to stderr, which Click'sCliRunnermerges intoresult.output. It already no-ops underCI=true, but a plain local run (network access + a version-check cache showing a newer release) gets the banner appended after JSON/CSV output, breakingjson.loads(result.output)indoctor --json,config show --json,ask --json,telemetry export --json/--csv, etc.test_dense.py's Ollama skip-guard only checks TCP connectivity, not whether thenomic-embed-textembed model is actually pulled. On a machine that runs Ollama for chat models but hasn't pulled that specific embed model, the retrieval-quality tests error out instead of skipping as the module docstring promises ("skipped if the server is unreachable").Changes
tests/conftest.py: new autouse fixture setsOPENJARVIS_NO_UPDATE_CHECK=1for the whole suite, so the update-check nag never fires locally either.tests/tools/storage/test_dense.py:_ollama_up()now hits/api/tagsand checks thatnomic-embed-textis present, instead of a baresocket.create_connectionport check.No production code changed — this is purely test-isolation. Confirmed green with the suite's canonical CI invocation:
Related issue
Unit tests failing
Test plan
uv run pytest tests/ -n auto -q -m "not live and not cloud and not hub"— 7201 passed, 56 skippeduv run ruff check tests/conftest.py tests/tools/storage/test_dense.py— cleanuv run ruff format --check tests/conftest.py tests/tools/storage/test_dense.py— cleantests/cli/test_version_check.py(which managesOPENJARVIS_NO_UPDATE_CHECKitself) still passes unaffected, since its local_clean_envfixture overrides the new global default.