Skip to content

docs: fix test command in CONTRIBUTING.md to use uv run#520

Merged
onuratakan merged 1 commit intomasterfrom
fix/contributing-test-command
Feb 5, 2026
Merged

docs: fix test command in CONTRIBUTING.md to use uv run#520
onuratakan merged 1 commit intomasterfrom
fix/contributing-test-command

Conversation

@IremOztimur
Copy link
Copy Markdown
Collaborator

@IremOztimur IremOztimur commented Feb 5, 2026

  • Fixed CONTRIBUTING.md test commands: Commands were missing uv run --all-extras prefix.

Full Changelog: master...upsonic/contributing-test-command

Pull Request:

@claude
Copy link
Copy Markdown

claude bot commented Feb 5, 2026

Review: docs: fix test command in CONTRIBUTING.md to use uv run

Summary

Good catch — the bare pytest commands would fail for anyone following the guide without first having manually installed all extras into the active venv. Adding uv run is the correct direction because it aligns with how this project manages dependencies.


Observations & Suggestions

1. --all-extras is heavier than necessary (minor)

--all-extras pulls in every optional dependency group (vector DBs, all storage backends, OCR, loaders, interfaces, etc.). The unit tests under tests/unit_tests/ are unlikely to need all of those. The setup section just above in the same file already recommends a targeted install:

uv pip install -e ".[vectordb,storage,models,embeddings,tools]"

Consider whether the test commands could mirror that scoping, e.g.:

uv run --extra tools pytest tests/unit_tests -v
uv run --extra tools pytest tests/unit_tests/tools/test_common_tools_duckduckgo.py -v
uv run --extra storage pytest tests/smoke_tests/memory -v

This would be faster and more instructive to contributors about which extras each test suite actually needs. That said, --all-extras is a safe default and definitely not wrong — it just trades install/resolve speed for simplicity.

2. Inconsistency with CLAUDE.md

CLAUDE.md (the project's own dev-guidance file) documents test commands as plain uv run pytest (no --all-extras):

# Run all tests
uv run pytest

# Run specific test directory
uv run pytest tests/rag/

The two files now give different invocation patterns for the same tool. It would be worth aligning them — either add --all-extras to CLAUDE.md as well, or decide on the minimal-extras approach for both.

3. Makefile smoke_tests target is still bare python3 -m pytest

The make smoke_tests target (and test_storage_only) in the Makefile use python3 -m pytest directly, bypassing uv run entirely. This is a separate pre-existing issue, but since this PR is touching the same "how to run tests" surface, it might be worth noting for a follow-up. The smoke tests section in CONTRIBUTING.md (make smoke_tests) inherits that inconsistency.

4. No issues with the changes themselves

The three-line diff is correct as written:

  • uv run properly invokes pytest inside uv's managed environment.
  • The placement and formatting are consistent with the surrounding markdown.
  • No typos or syntax issues.

Verdict

The fix is valid and an improvement over the status quo. The suggestions above are about scope/consistency rather than correctness. Happy to approve as-is if the maintainers are fine with the --all-extras blanket approach; otherwise the extras-scoping and CLAUDE.md alignment are straightforward follow-ups.

@onuratakan onuratakan merged commit 69b54a0 into master Feb 5, 2026
5 checks passed
@onuratakan onuratakan deleted the fix/contributing-test-command branch February 5, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants