Skip to content

Commit b05436b

Browse files
committed
Move tests/common/fixtures/* into conftest.py
Move all the pytest fixtures defined in `tests/common/fixtures/*.py` files into `conftest.py` files. Context ------- pytest provides a method of organising fixtures by putting them into hierarchical/local `conftest.py` files: tests/ conftest.py # Fixtures defined in this file are available to all tests. unit/ conftest.py # Available to all unit tests. functional/ conftest.py # Available to all functests. foo/ conftest.py # Available to all tests in foo/ and below. pytest automatically discovers and registers fixtures defined in `conftest.py` files and makes them available to other files (both test files and lower `conftest.py` files) for fixture injection _without those files having to import the fixtures_. See: https://docs.pytest.org/en/stable/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files A downside of this is that `conftest.py` files could get quite large but it shouldn't get too bad: you shouldn't have too many shared fixtures anyway. At Hypothesis, [we thought we knew better](#5002). We allowed this downside to tempt us to create `tests/common/fixtures/*.py` files, allowing us to split our fixtures up into multiple separate files. Our `conftest.py` files then needed to import these fixtures from `tests.common.fixtures` so that pytest would register them. Problem ------- Ah, hubris. In an upcoming future PR I need to have a common `es_client` fixture that's shared between the unittests and the functests, and I need to override that fixture to add some custom behaviour for the unittests only. Here's how you'd normally do it with `conftest.py` files, this works: # tests/conftest.py @pytest.fixture def es_client(): client = _es_client() yield client client.close() # tests/unit/conftest.py @pytest.fixture def es_client(es_client): yield es_client `tests/conftest.py` defines an `es_client` fixture that's available to all test files and `conftest.py` files in the `tests/` directory, both unittests and functests. `tests/unit/conftest.py` then overrides the `es_client` fixture with its own `es_client` fixture that depends on the higher-level `es_client` fixture and adds some additional teardown. This is normal pytest fixture overriding. With our `tests/common/fixtures/` directory this doesn't work: # tests/common/fixtures/elasticsearch.py @pytest.fixture def es_client(): client = _es_client() yield client client.close() # tests/unit/conftest.py from tests.common.fixtures.elasticsearch import es_client @pytest.fixture def es_client(es_client): yield es_client clear_search_index() What happens is: 1. `tests/unit/conftest.py` has to import the `es_client` fixture from `tests/common/fixtures/elasticsearch.py`, otherwise pytest won't discover the fixture. 2. When `tests/unit/conftest.py` then defines its own `es_client` function that overrides the name `es_client` in the module's namespace, so now pytest will _not_ discover the original `es_client` function that was imported from the common directory. 3. When pytest sees an `es_client` fixture that depends on a fixture named `es_client`, it thinks the fixture is trying to depend on itself and crashes with a circular fixture dependency error. Solution -------- Get rid of the `tests/common/fixtures/` directory. Just do the normal pytest thing and move all these fixtures into `tests/conftest.py`. When any other files (including lower `conftest.py` files) want to use any of these common fixtures they can just do so via pytest fixture injection without needing to import anything. There were some fixtures in `tests/common/fixtures/services.py` that were actually only used in the unittests. These have been moved into `tests/unit/h/conftest.py`. It *is* possible to have our cake and eat it here: you can put your fixtures in separate files (thus avoiding have them all in one large file) and then import them all into the top-level `tests/conftest.py` file and all other files use the fixtures via injection. But I think it's simpler just to put shared fixtures in `conftest.py` files.
1 parent c615572 commit b05436b

File tree

10 files changed

+406
-472
lines changed

10 files changed

+406
-472
lines changed

.cookiecutter/includes/tox/passenv

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,4 @@ dev: ORCID_HOST
1414
dev: ORCID_CLIENT_ID
1515
dev: ORCID_CLIENT_SECRET
1616
{tests,functests}: DATABASE_URL
17-
{tests,functests}: ELASTICSEARCH_URL
1817
functests: BROKER_URL

.cookiecutter/includes/tox/setenv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ dev: H_API_AUTH_COOKIE_SECRET_KEY = {env:H_API_AUTH_COOKIE_SECRET_KEY:"dev_h_api
1111
dev: H_API_AUTH_COOKIE_SALT = {env:H_API_AUTH_COOKIE_SALT:"dev_h_api_auth_cookie_salt"}
1212
dev: REPLICA_DATABASE_URL = {env:DATABASE_URL:postgresql://postgres@localhost/postgres}
1313
dev: MAILCHIMP_USER_ACTIONS_SUBACCOUNT = {env:MAILCHIMP_USER_ACTIONS_SUBACCOUNT:devdata}
14+
tests,functests: ELASTICSEARCH_URL = {env:ELASTICSEARCH_URL:http://localhost:9200}
1415
tests: ELASTICSEARCH_INDEX = {env:ELASTICSEARCH_INDEX:hypothesis-tests}
1516
functests: ELASTICSEARCH_INDEX = {env:ELASTICSEARCH_INDEX:hypothesis-functests}
1617
{tests,functests}: AUTHORITY = {env:AUTHORITY:example.com}

tests/common/fixtures/__init__.py

Whitespace-only changes.

tests/common/fixtures/elasticsearch.py

Lines changed: 0 additions & 84 deletions
This file was deleted.

0 commit comments

Comments
 (0)