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

Download example databases for use in CI tests #608

Merged
merged 17 commits into from
Jan 2, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ jobs:
shell: bash -l {0}
run: |
set -vxeuo pipefail
coverage run -m pytest -v -m "not slow"
coverage run -m pytest -v
coverage report
env:
# Provide test suite with a PostgreSQL database to use.
Expand Down
4 changes: 4 additions & 0 deletions continuous_integration/docker-configs/postgres-ci.dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
FROM docker.io/postgres
# Uses the initialization scripts (https://hub.docker.com/_/postgres/)
# Creates Database automatically from SQL file
COPY postgres-ci-db.sql /docker-entrypoint-initdb.d/init-user-db.sql
3 changes: 0 additions & 3 deletions pytest.ini
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ log_cli = 1
log_cli_level = WARNING
log_cli_format = %(asctime)s [%(levelname)8s] %(message)s (%(filename)s:%(lineno)s)
log_cli_date_format=%Y-%m-%d %H:%M:%S
addopts = --strict-markers -m 'not slow'
markers =
slow: marks tests as slow (deselect with '-m "not slow"')
85 changes: 66 additions & 19 deletions tiled/_tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import tempfile
from pathlib import Path

import asyncpg
import pytest
import pytest_asyncio
from sqlalchemy.ext.asyncio import create_async_engine

from .. import profiles
from ..catalog import from_uri, in_memory
Expand Down Expand Up @@ -105,29 +107,74 @@ def poll_enumerate():
TILED_TEST_POSTGRESQL_URI = os.getenv("TILED_TEST_POSTGRESQL_URI")


@pytest_asyncio.fixture(params=["sqlite", "postgresql"])
async def adapter(request, tmpdir):
@pytest_asyncio.fixture
async def postgresql_adapter(request, tmpdir):
"""
Adapter instance

Note that startup() and shutdown() are not called, and must be run
either manually (as in the fixture 'a') or via the app (as in the fixture 'client').
"""
if request.param == "sqlite":
adapter = in_memory(writable_storage=str(tmpdir))
if not TILED_TEST_POSTGRESQL_URI:
raise pytest.skip("No TILED_TEST_POSTGRESQL_URI configured")
# Create temporary database.
async with temp_postgres(TILED_TEST_POSTGRESQL_URI) as uri_with_database_name:
# Build an adapter on it, and initialize the database.
adapter = from_uri(
uri_with_database_name,
writable_storage=str(tmpdir),
init_if_not_exists=True,
)
yield adapter
elif request.param == "postgresql":
if not TILED_TEST_POSTGRESQL_URI:
raise pytest.skip("No TILED_TEST_POSTGRESQL_URI configured")
# Create temporary database.
async with temp_postgres(TILED_TEST_POSTGRESQL_URI) as uri_with_database_name:
# Build an adapter on it, and initialize the database.
adapter = from_uri(
uri_with_database_name,
writable_storage=str(tmpdir),
init_if_not_exists=True,
)
yield adapter
await adapter.shutdown()
else:
assert False


@pytest_asyncio.fixture
async def postgresql_with_example_data_adapter(request, tmpdir):
"""
Adapter instance

Note that startup() and shutdown() are not called, and must be run
either manually (as in the fixture 'a') or via the app (as in the fixture 'client').
"""
if not TILED_TEST_POSTGRESQL_URI:
raise pytest.skip("No TILED_TEST_POSTGRESQL_URI configured")
DATABASE_NAME = "example_data"
uri = TILED_TEST_POSTGRESQL_URI
if uri.endswith("/"):
uri = uri[:-1]
uri_with_database_name = f"{uri}/{DATABASE_NAME}"
engine = create_async_engine(uri_with_database_name)
try:
async with engine.connect():
pass
except asyncpg.exceptions.InvalidCatalogNameError:
raise pytest.skip(
f"PostgreSQL instance contains no database named {DATABASE_NAME!r}"
)
adapter = from_uri(
uri_with_database_name,
writable_storage=str(tmpdir),
)
yield adapter


@pytest_asyncio.fixture
async def sqlite_adapter(request, tmpdir):
"""
Adapter instance

Note that startup() and shutdown() are not called, and must be run
either manually (as in the fixture 'a') or via the app (as in the fixture 'client').
"""
yield in_memory(writable_storage=str(tmpdir))


@pytest.fixture(params=["sqlite_adapter", "postgresql_adapter"])
def adapter(request):
"""
Adapter instance

Note that startup() and shutdown() are not called, and must be run
either manually (as in the fixture 'a') or via the app (as in the fixture 'client').
Comment on lines +195 to +196
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ease of use, it seems like it would be convenient to keep startup() and shutdown() in the fixtures. Is it straightforward to add a guard to startup() that checks whether it has already been called or whether an app is running?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do dislike this structure and would welcome suggestions to improve it.

The problem is that startup must be called on the same thread where the application will run. If this adapter is going to be used by the TestClient, via Context.from_app(build_app(adapter)), a background thread is created at that point and startup()` needs to be run on that thread.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. In that case, I think the solution using the fixture a is probably already optimal.


One could of course add fixture b (or equivalent) for test_metadata_index_is_used(b) -- if you expect additional tests that would make use of postgresql_with_example_data_adapter:

@pytest_asyncio.fixture
async def b(postgresql_with_example_data_adapter):
    "Raw adapter, not to be used within an app becaues it is manually started and stopped."
    adapter = postgresql_with_example_data_adapter
    await adapter.startup()
    yield adapter
    await adapter.shutdown()

However, a DRYer and more composable version might look like this...

@pytest.mark.parametrize("a", ["postgresql_with_example_data_adapter"], indirect=True)
@pytest.mark.asyncio
async def test_metadata_index_is_used(a):
    # a, a.startup(), a.shutdown() are no longer needed
    ...

Marking the parameter as indirect will override the argument passed to a parameterized fixture. See "Indirect parametrization" | parametrize | and especially this informative example.

"""
yield request.getfixturevalue(request.param)
19 changes: 5 additions & 14 deletions tiled/_tests/test_catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,24 +156,14 @@ async def test_search(a):
assert await d.search(Eq("number", 12)).keys_range(0, 5) == ["c"]


@pytest.mark.slow
@pytest.mark.asyncio
async def test_metadata_index_is_used(a):
for i in range(10000):
await a.create_node(
metadata={
"number": i,
"number_as_string": str(i),
"nested": {"number": i, "number_as_string": str(i), "bool": bool(i)},
"bool": bool(i),
},
specs=[],
structure_family="array",
)
# Check that an index (specifically the 'top_level_metdata' index) is used
async def test_metadata_index_is_used(postgresql_with_example_data_adapter):
a = postgresql_with_example_data_adapter # for succinctness below
# Check that an index (specifically the 'top_level_metadata' index) is used
# by inspecting the content of an 'EXPLAIN ...' query. The exact content
# is intended for humans and is not an API, but we can coarsely check
# that the index of interest is mentioned.
await a.startup()
with record_explanations() as e:
results = await a.search(Key("number_as_string") == "3").keys_range(0, 5)
assert len(results) == 1
Expand All @@ -200,6 +190,7 @@ async def test_metadata_index_is_used(a):
)
assert len(results) == 1
assert "top_level_metadata" in str(e)
await a.shutdown()


@pytest.mark.asyncio
Expand Down