schireson / pytest-mock-resources

Pytest Fixtures that let you actually test against external resource (Postgres, Mongo, Redshift...) dependent code.
https://pytest-mock-resources.readthedocs.io/en/latest/quickstart.html
MIT License
179 stars 19 forks source link

feat: Add mysql support #95

Closed michaelbukachi closed 4 years ago

michaelbukachi commented 4 years ago

@DanCardin @oakhan3 Do I need to update the documentation? Also, I wasn't able to add tests for mysql in relational/test_generic.py and test_rows.py since mysql doesn't have a default public schema. Before we can add tests we'll have create a public schema and make it the default. Should I go with this approach?

michaelbukachi commented 4 years ago

It seems the CI is installing new dependencies.

DanCardin commented 4 years ago

@michaelbukachi, the existing tests are a bit presumptuous about public being the default schema. I dont think you ought to stray from the default mysql behavior actually.

For test_rows.py, if I'm reading it right, there's not really any need for it to have the public schema specified on the model.

For test_generic.py, I think you can just test the actually generic bits, i.e. that tables= works with the Model, Model.__table__, and 'tablename', which is the more important bit of that file's tests.

The public schema tests was more to verify we could postgres-alike with sqlite, as a lite version that doesn't require docker. So we should probably ultimately alter the sqlite fixture to more flexibly mimic the behaviors as explicit modes (including an unchanged mode).

DanCardin commented 4 years ago

I'm not sure what's going on with poetry/CI. wanna try re-triggering?

michaelbukachi commented 4 years ago

I'm not sure what's going on with poetry/CI. wanna try re-triggering?

I'm also getting the same errors on my own CI account. Btw, do you have a guide for setting up the environment for tests locally? Some tests don't pass when I use make especially test-parallel

DanCardin commented 4 years ago

I wouldn't expect you to need to do much more than run poetry install with the extras (i.e. make install inside a virtualenv, have docker running, and run the tests.

Are you seeing the same sort of docker issues you saw on ubuntu in your other issue?

DanCardin commented 4 years ago

For what it's worth, I checked out your branch from your fork, and the tests seem to be passing for me.

michaelbukachi commented 4 years ago

For what it's worth, I checked out your branch from your fork, and the tests seem to be passing for me.

Interesting. A question. If the docker container is already running. Will it affect the tests?

DanCardin commented 4 years ago

it shouldn't. That's i.e. what the pmr utility is doing. Pre-starting the container to avoid the cost of startup time. It just needs to be some postgres (or whatever resource) available on the expected port with the expected credentials.

michaelbukachi commented 4 years ago

@DanCardin Finally fixed it. The issue was this statement in test_statements.py:

select table_name from information_schema.views order by table_name

Apparently, mysql does not limit the views to the current schema. It returns all views in all schemas. Changing the mysql statement to:

        SELECT table_name
        FROM INFORMATION_SCHEMA.views
        WHERE table_name in ('cool_view', 'cool_view_2')
        AND table_schema = (select database())
        ORDER BY table_name

solves it. An interesting thing to note is that this bug could only be caught by running pmr mysql first before running the tests.

DanCardin commented 4 years ago

Yea that's a weird sort of edge case that you'd have to be aware of when using (the default) session scope fixture which shares the container. SELECT datname FROM pg_database for postgres would similarly show all the things.

I'll create an issue to document this, as it's possible to work around. Unfortunately, it's not really something we can produce good errors for.

michaelbukachi commented 4 years ago

@oakhan3 On it.