Skip to content

fix(wren): keep just test collecting without the postgres extra - #2801

Merged
goldmedal merged 1 commit into
Canner:mainfrom
lihongyuan99:test/postgres-importorskip
Oct 20, 2026
Merged

goldmedal merged 1 commit into
Canner:mainfrom
lihongyuan99:test/postgres-importorskip

Conversation

@lihongyuan99

@lihongyuan99 lihongyuan99 commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Summary

tests/connectors/test_postgres.py:16 did import psycopg at module level, but psycopg
is only in the optional postgres extra (core/wren/pyproject.toml:
postgres = ["psycopg[binary]>=3"]) and not in the dev group that just install
(uv sync) resolves. On the setup path CONTRIBUTING.md prescribes for core/wren
(just install && just test), that import is an unhandled ModuleNotFoundError during
collection, which aborts the whole pytest session — just test runs zero tests.
This makes the module skip itself when the extra is absent, the way
tests/connectors/test_mssql.py:19 already does for pyodbc.

What failure does this repair?

Reproduced on main @ fedf182 in a just install environment (core/wren, Python
3.12.12; psycopg not importable, testcontainers/duckdb/orjson/pyarrow present):

$ uv run --no-sync pytest tests/ --collect-only -q
tests/connectors/test_postgres.py:16: in <module>
    import psycopg
E   ModuleNotFoundError: No module named 'psycopg'
ERROR tests/connectors/test_postgres.py
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
1922 tests collected, 1 error in 30.07s
$ echo $?
2

Interrupted: 1 error during collection means no test file runs at all, including the
service-free unit tree. just test is uv run --no-sync pytest tests/ -v, so this is the
documented entry point failing, not an exotic selection.

The skip has to be placed before the first-party imports, not just before psycopg's
old position: src/wren/connector/postgres.py:19 imports psycopg at module level too, so
from wren.connector.postgres import ... (line 24) would raise the same error one line
later. That is why the following import block carries # noqa: E402, mirroring
test_mssql.py:25-31.

How is it tested?

command (from core/wren/) before after
uv run --no-sync pytest tests/ --collect-only -q exit 2, 1922 tests collected, 1 error, session interrupted exit 0, 1922 tests collected in 0.79s
uv run --no-sync pytest tests/connectors/test_postgres.py -q collection error 1 skipped in 0.07s
uv run --no-sync pytest tests/unit/ -q --ignore=tests/unit/test_memory.py --ignore=tests/unit/test_mcp_server.py 1 failed, 1519 passed, 2 skipped in 27.65s 1 failed, 1519 passed, 2 skipped in 27.56s
uv run --no-sync pytest tests/connectors/test_postgres.py --collect-only -q with psycopg[binary] installed — 37 tests collected
uv run --no-sync ruff check tests/connectors/test_postgres.py 6 findings (3 E402, 3 PLC0415) 6 findings (same 6)
uv run --no-sync ruff format --check tests/connectors/test_postgres.py already formatted already formatted

The single unit failure is pre-existing on main under this exact selection
(test_cloud.py::test_create_refuses_without_a_git_identity_before_creating_anything);
it was measured on the untouched baseline above. The changed file itself is exercised by
CI's datasource tests matrix job, which installs the extra — verified by the
37 tests collected row, i.e. the skip is a no-op there. I did not run the
container-backed tests: this machine has no running Docker daemon.

No test file is added; the red/green is the exit status of the existing collection.

Duplicate check

Summary by CodeRabbit

  • Tests
    • PostgreSQL connector tests now skip cleanly when the optional PostgreSQL driver is unavailable, with a clear indication that the PostgreSQL extra is needed. When the driver is installed, the tests remain available as before. This prevents test runs from failing solely because an optional connector dependency is absent in the test environment.

tests/connectors/test_postgres.py imported psycopg at module level, but psycopg ships only in the optional postgres extra, not in the dev group that `just install` resolves. On the documented setup path the import aborts collection of `pytest tests/`, so `just test` runs zero tests. Skip the module when the extra is absent, the way test_mssql.py already does for pyodbc.
@github-actions github-actions Bot added python Pull requests that update Python code core labels Oct 11, 2026
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 71a31fa9-ebf3-4517-a2ac-47f3181058a5

📥 Commits

Reviewing files that changed from the base of the PR and between fedf182 and 5868e3e.


📒 Files selected for processing (1)
  • core/wren/tests/connectors/test_postgres.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.



Walkthrough

The PostgreSQL test module now checks for psycopg before loading PostgreSQL-specific imports. If the package is unavailable, pytest skips the module with a reason that identifies the PostgreSQL extra.

Changes

PostgreSQL test collection

Layer / File(s) Summary
Optional dependency check and import order
core/wren/tests/connectors/test_postgres.py
The module uses pytest.importorskip to skip tests when psycopg is unavailable. PostgreSQL-specific imports follow the check.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: bartok9

Merge Risk: ⚪ Minimal · up to 5868e

Ordinary test collection can skip PostgreSQL-specific tests when the optional dependency is absent; no material merge-blocking risk is identified.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check Passed The description includes all required sections. It provides a clear summary, the reproduced failure and actual error output, test commands and results, CI coverage details, and a duplicate check.
Title check Passed The title clearly and concisely describes the main change: allowing test collection without the optional PostgreSQL extra.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

I’m a rabbit; I check for psycopg with care.
If it is missing, I skip this test lair.
PostgreSQL imports wait their turn,
While tidy test collections run.
I nibble a leaf and hop along.

Comment @coderabbitai help to get the list of available commands.

@goldmedal goldmedal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @lihongyuan99 👍

@goldmedal
goldmedal merged commit 5a287c5 into Canner:main Oct 20, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants