Skip to content

ensure new connection for each test sql#9356

Merged
Light2Dark merged 1 commit into
mainfrom
sham/fix-test-get-databases-error
Apr 24, 2026
Merged

ensure new connection for each test sql#9356
Light2Dark merged 1 commit into
mainfrom
sham/fix-test-get-databases-error

Conversation

@Light2Dark
Copy link
Copy Markdown
Collaborator

@Light2Dark Light2Dark commented Apr 24, 2026

📝 Summary

Supersedes #9352 by ensuring each test runs in a duckdb connection that closes automatically.

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

Summary by cubic

Use a fresh in-memory duckdb connection per test to isolate state and prevent flaky SQL error-handling tests. Tests now pass engine=duckdb_conn and call duckdb_conn.sql(...), so connections close automatically after each test.

  • Bug Fixes
    • Add pytest fixture duckdb_conn that creates duckdb.connect(':memory:') and closes it after each test.
    • Replace implicit engine/global duckdb.sql(...) with explicit engine=duckdb_conn and duckdb_conn.sql(...).
    • Stabilizes tests for missing tables/columns, syntax errors, hints, and long-statement truncation.

Written for commit 359e017. Summary will update on new commits.

Copilot AI review requested due to automatic review settings April 24, 2026 04:53
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Apr 24, 2026 4:53am

Request Review

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

@Light2Dark Light2Dark added the internal A refactor or improvement that is not user facing label Apr 24, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the DuckDB SQL error-handling tests to ensure each test runs against its own in-memory DuckDB connection, preventing cross-test table leakage and improving test isolation (superseding #9352’s table-cleanup approach).

Changes:

  • Add a per-test duckdb_conn fixture that creates/closes an isolated in-memory DuckDB connection.
  • Update DuckDB-related tests to use engine=duckdb_conn (and duckdb_conn.sql(...)) instead of the process-wide default connection.

Comment thread tests/_sql/test_sql_error_handling.py
Comment thread tests/_sql/test_sql_error_handling.py
@Light2Dark Light2Dark requested a review from akshayka April 24, 2026 05:40
@Light2Dark Light2Dark enabled auto-merge (squash) April 24, 2026 05:41
Copy link
Copy Markdown
Contributor

@akshayka akshayka left a comment

Choose a reason for hiding this comment

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

Thank you, much nicer :)

@Light2Dark Light2Dark merged commit e6b956a into main Apr 24, 2026
53 of 58 checks passed
@Light2Dark Light2Dark deleted the sham/fix-test-get-databases-error branch April 24, 2026 05:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal A refactor or improvement that is not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants