Skip to content

TST: use fixtures for some sql database connections #42976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Dec 22, 2021

Conversation

fangchenli
Copy link
Member

@fangchenli fangchenli commented Aug 11, 2021

xref #40686

This PR:

  1. Create fixtures for all supported SQL connections, including sqlalchemy engine/connection and sqlite3 connection. Setup and cleanup are handled within fixtures.
  2. Migrate several common tests to use connection fixtures. They serve as examples for future migrations.

We probably should put all the fixtures in a separate conftest.py file. But since there are unmerged PRs related to this test file, I kept everything in one place for now to avoid conflict.

@fangchenli fangchenli added Testing pandas testing functions or related to the test suite IO SQL to_sql, read_sql, read_sql_query labels Aug 11, 2021
@fangchenli fangchenli marked this pull request as draft August 11, 2021 02:31
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Sep 30, 2021
@mroeschke
Copy link
Member

Thanks for the PR but appears that it has gone stale. Feel free to reopen if you have time to continue working on it!

@mroeschke mroeschke closed this Nov 6, 2021
@fangchenli fangchenli reopened this Nov 29, 2021
@fangchenli fangchenli removed the Stale label Nov 29, 2021
@@ -367,6 +375,270 @@ def test_frame3():
return DataFrame(data, columns=columns)


@pytest.fixture
Copy link
Member

Choose a reason for hiding this comment

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

Does it makes sense for the the engine and connection fixtures to be scope="module" so they are not repeatedly created and destroyed?

Copy link
Member Author

Choose a reason for hiding this comment

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

The fixtures also do cleanups after each test. So it's better to have a new engine/conn for each test. Also, they don't seem to add much overhead.

@jreback
Copy link
Contributor

jreback commented Dec 20, 2021

@fangchenli happyt o take this if you can fixup

@fangchenli fangchenli marked this pull request as ready for review December 21, 2021 16:37
@mroeschke mroeschke added this to the 1.4 milestone Dec 21, 2021
@jreback jreback merged commit 4735c0f into pandas-dev:master Dec 22, 2021
@jreback
Copy link
Contributor

jreback commented Dec 22, 2021

thanks @fangchenli

@fangchenli fangchenli deleted the fixtures-sql-conn branch November 18, 2022 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO SQL to_sql, read_sql, read_sql_query Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants