Skip to content
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

TST: Extract methods from TestXMySQL and TestXSQLite and remove #20536

Closed
TomAugspurger opened this issue Mar 29, 2018 · 6 comments
Closed

TST: Extract methods from TestXMySQL and TestXSQLite and remove #20536

TomAugspurger opened this issue Mar 29, 2018 · 6 comments
Labels
IO SQL to_sql, read_sql, read_sql_query Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite

Comments

@TomAugspurger
Copy link
Contributor

I haven't looked too closely, but in https://github.com/pandas-dev/pandas/pull/13611/files the TestXMySql tests were unconditionally skipped.

IIUC, these are the test using a DBAPI2 Connection object, which is deprecated? We should remove these tests if they aren't being run.

@TomAugspurger TomAugspurger added Testing pandas testing functions or related to the test suite IO SQL to_sql, read_sql, read_sql_query labels Mar 29, 2018
@jorisvandenbossche
Copy link
Member

IIUC, these are the test using a DBAPI2 Connection object, which is deprecated?

It's no longer deprecated, but actually removed (for mysql writing) or accepted (for sqlite, and in practice also for reading query's)

We should remove these tests if they aren't being run.

Yeah, IIRC those tests were old tests from before the sqlalchemy refactor, and kept for "we should actually clean up the tests (go through them, and keep the useful bits in case it is not yet covered in the other tests), but that is some work, so just leave them there for now" reasons

@TomAugspurger
Copy link
Contributor Author

Gotcha, so the TODO here is to pick out the interesting tests, and move them to the _TestMySQLAlchemy class?:

@jorisvandenbossche
Copy link
Member

Yep (but it might be there is actually nothing interesting)

@jorisvandenbossche
Copy link
Member

And the same is true actually for TestXSQLite (which is not skipped), also for those it would be nice to better integrate them with the rest of the tests

@TomAugspurger TomAugspurger changed the title TST: Remove or enable TestXMySQL tests? TST: Extract methods from TestXMySQL and TestXSQLite and remove Mar 29, 2018
@TomAugspurger
Copy link
Contributor Author

One potential strategy for discovering which lines are tested in just TestX*:

  1. Run just the TestSQLiteAlchemy tests with covereage
  2. Run just the TestXSQLite tests with coverage

dif the two coverage reports (not sure if there's a tool to do this, or if you would do it manually).

@mroeschke
Copy link
Member

I think the precursor ADBC work to refactor the sql tests addressed this so closing

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 Refactor Internal refactoring of code Testing pandas testing functions or related to the test suite
Projects
None yet
Development

No branches or pull requests

4 participants