TST: Refactoring backend tests#2566
TST: Refactoring backend tests#2566jreback merged 6 commits intoibis-project:masterfrom datapythonista:backend_tests
Conversation
datapythonista
added
refactor
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
looks good. just a couple of questions.
| import ibis.expr.datatypes as dt | ||
| import ibis.expr.types as ir | ||
| from ibis.tests.backends import Spark | ||
| from ibis.backends.spark.tests.conftest import SparkTest |
There was a problem hiding this comment.
hmm isn't this a pytest warning? we generally don't want to do this
There was a problem hiding this comment.
e.g. just put the methods in another file (not conftest)
There was a problem hiding this comment.
I'm ok changing this, but pytest is happy, no warnings. And in several cases there is already related code in conftest.py. For example, in bigquery the connect methods of the tests class is already in conftest. And in omnisci and others, the parameters for the connection are loaded in conftest and reused for both the test class and for things in conftest. That's why I thought that conftest is a better place (even if in couple of instances like this one conftest needs to be imported). And also because the class is not tests themselves but more the configuration (even if not pytest specific).
So, to me it makes a bit more sense to leave the test classes in conftest.py, but if you have a strong opinion on having it separate, I'm ok with that, I'll move it next week. Let me know if you have a preference for the file where to save it (since I need to create 12 of them, to not waste time with renames later).
There was a problem hiding this comment.
nope it's fine
in pandas we prohibit this but maybe that's some enforced outside of pytest i don't remember
| ) | ||
| ) | ||
|
|
||
| for marker in list(item.iter_markers(name="xpass_backends")): |
There was a problem hiding this comment.
Not really sure. I opened #2570, and will have a look next week. I don't want to remove of change any code in this PR, and just have the code copied as is, so we can track the changes easier later.
|
thanks @datapythonista very nice makes testing more obvious - happy to have it at some point would be great to describe this testing paradigm in a readme / docs |
Great idea. I created #2571. Will probably wait a bit for it, since I think we can still make things a bit simpler first, but will be adding docs for it. |
|
yes for sure |
3 participants
Better structure of the backend tests. The diff is big, but there are just two changes:
ibis/tests/alltoibis/backends/testsibis/tests/backends.pytoibis/backends/<backend>/tests/conftest.pyThe idea is to make the backend tests not know about the specific backends, and have a better decouping of things, so backends that are in a separate repo can also be tested.