Test musicbrainz db methods against a real musicbrainz sample database #44
Conversation
| mb_artist.mb_session = MagicMock() | ||
| self.mock_db = mb_artist.mb_session.return_value.__enter__.return_value | ||
| self.artist_query = self.mock_db.query.return_value.options.return_value.filter.return_value.all | ||
| @pytest.mark.database |
There was a problem hiding this comment.
in pytest, a mark allows you to tag a particular test, and ask it to only run specific marks, or to exclude them. By marking db tests, we can run two separate sets of tests, the regular unit tests which run quickly, and the database ones, which might be slower
| self.mock_db = mb_artist.mb_session.return_value.__enter__.return_value | ||
| self.artist_query = self.mock_db.query.return_value.options.return_value.filter.return_value.all | ||
| @pytest.mark.database | ||
| class TestArtist: |
There was a problem hiding this comment.
pytest doesn't require tests to inherit from unittest.TestCase. Instead, it just has to follow a specific naming pattern (TestX...)
| @@ -0,0 +1,8 @@ | |||
| import pytest | |||
There was a problem hiding this comment.
conftest.py is a magic pytest file that is automatically loaded. it's a bit weird, but this is how you do it
|
|
||
|
|
||
| @pytest.fixture(scope="session") | ||
| def engine(): |
There was a problem hiding this comment.
This is a pytest fixture. It allows you to inject specific data into a test. It's used by adding the name of the fixture as a parameter to a test. It's pretty magic, try not to think about it
|
|
||
| def test_get_by_id(self): | ||
| self.artist_query.return_value = [artist_linkin_park] | ||
| def test_get_by_id(self, engine): |
There was a problem hiding this comment.
this is how you specify that a test requires a fixture. By marking that this test needs the sqlalchemy engine, it'll connect to the database before running this test. Fixtures are nicer than setUp methods, because you can selectively apply them to only the methods that you need
|
|
||
| def test_get_by_id(self): | ||
| self.artist_query.return_value = [artist_linkin_park] | ||
| def test_get_by_id(self, engine): | ||
| artist = mb_artist.get_artist_by_id("f59c5520-5f46-4d2c-b2c4-822eabf53419") |
There was a problem hiding this comment.
Luckily, Linkin Park was already in the test data
| artist = mb_artist.get_artist_by_id("f59c5520-5f46-4d2c-b2c4-822eabf53419") | ||
| self.assertDictEqual(artist, { | ||
| assert artist == { |
There was a problem hiding this comment.
pytest uses assert, instead of assert methods on a parent class.
| @@ -0,0 +1,94 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
These files are copied from the musicbrainz-docker project. Eventually we should be able to merge these projects together to remove the code duplication, but for now this is the easiest way of getting a sample database dump loaded into the musicbrainz-test-database image
| test: | ||
| build: | ||
| context: .. | ||
| dockerfile: ./test/Dockerfile.py3 |
There was a problem hiding this comment.
currently only testing on py3. Not sure if we should also include py2. It's probably not worth it since we don't use this db access in AB at the moment.
|
|
||
| @pytest.fixture(scope="session") | ||
| def engine(): | ||
| init_db_engine("postgresql://musicbrainz@musicbrainz_db/musicbrainz_db") |
There was a problem hiding this comment.
this shouldn't be hard-coded.
There are some pytest plugins that allow for transactional tests, e.g. https://github.com/jeancochrane/pytest-flask-sqlalchemy, however I'm not sure that it's necessary, as we're only using the database in read-only mode.
|
I have updated the tests to use the test database at https://github.com/amCap1712/brainzutils-python/tree/mbdb-test. I cannot add commits to this PR but cherry-picking the latest 3 commits should do. I modified the .travis.yml to run tests only with python 3. With python 2, I get weird encoding errors due to the test data used. If AB is going to be updated to Python 3 soon, I don't think we need to modify the tests to make Python 2 happy. |
|
Thanks, I applied your commits to this branch. |
We found some errors in the musicbrainz_db module due to the mocks in the tests not accurately reflecting what is actually returned by a query: https://github.com/metabrainz/brainzutils-python/blame/a73309306697cd08bc4d0bad2e296cc61c993713/brainzutils/musicbrainz_db/utils.py#L80-L87
This is an experimental setup to include a copy of the musicbrainz sample database, and use it when running tests that use mbdata.
Currently, a multi-step process. To build,
some inline comments in the PR
cc @amCap1712