Skip to content

Commit

Permalink
Run tests using pytest-xdist to speed up our test runs.
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathangreen committed May 1, 2024
1 parent 6ebedf2 commit 9488ba1
Show file tree
Hide file tree
Showing 21 changed files with 540 additions and 464 deletions.
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ x-cm-variables: &cm
PALACE_CELERY_CLOUDWATCH_STATISTICS_DRYRUN: "true"

# Set up the environment variables used for testing as well
SIMPLIFIED_TEST_DATABASE: "postgresql://palace:test@pg:5432/circ"
PALACE_TEST_DATABASE_URL: "postgresql://palace:test@pg:5432/circ"
PALACE_TEST_SEARCH_URL: "http://os:9200"
PALACE_TEST_MINIO_ENDPOINT_URL: "http://minio:9000"
PALACE_TEST_MINIO_USER: "palace"
Expand Down
37 changes: 36 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@ exclude_also = [
'^\s*pass\s*$',
'^\s*raise NotImplementedError\s*$',
]
include_namespace_packages = true

[tool.coverage.run]
branch = true
concurrency = ["multiprocessing", "thread"]
parallel = true
relative_files = true
source = ["palace.manager"]
source = ["src"]

[tool.isort]
known_first_party = ["palace"]
Expand Down Expand Up @@ -286,6 +289,7 @@ pytest-alembic = "^0.11.0"
pytest-celery = "^0.0.0"
pytest-cov = "^5.0.0"
pytest-timeout = "*"
pytest-xdist = "^3.5.0"
requests-mock = "1.12.1"
types-aws-xray-sdk = "^2.11.0.13"
types-Flask-Cors = "^4.0.0"
Expand All @@ -306,6 +310,8 @@ psycopg2 = "~2.9.5"
addopts = [
"--cov",
"--cov-report=xml",
"--dist=load",
"--numprocesses=auto",
"--strict-markers",
]
markers = [
Expand Down
35 changes: 7 additions & 28 deletions src/palace/manager/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class Configuration(ConfigurationConstants):
log = logging.getLogger("Configuration file loader")

# Environment variables that contain URLs to the database
DATABASE_TEST_ENVIRONMENT_VARIABLE = "SIMPLIFIED_TEST_DATABASE"
DATABASE_PRODUCTION_ENVIRONMENT_VARIABLE = "SIMPLIFIED_PRODUCTION_DATABASE"

# Environment variable for Overdrive fulfillment keys
Expand Down Expand Up @@ -72,46 +71,26 @@ class Configuration(ConfigurationConstants):

@classmethod
def database_url(cls):
"""Find the database URL configured for this site.
For compatibility with old configurations, we will look in the
site configuration first.
If it's not there, we will look in the appropriate environment
variable.
"""

# To avoid expensive mistakes, test and production databases
# are always configured with separate keys. The TESTING variable
# controls which database is used, and it's set by the
# package_setup() function called in every component's
# tests/__init__.py.
test = os.environ.get("TESTING", False)
if test:
environment_variable = cls.DATABASE_TEST_ENVIRONMENT_VARIABLE
else:
environment_variable = cls.DATABASE_PRODUCTION_ENVIRONMENT_VARIABLE

url = os.environ.get(environment_variable)
"""Find the database URL configured for this site."""
url = os.environ.get(cls.DATABASE_PRODUCTION_ENVIRONMENT_VARIABLE)

Check warning on line 75 in src/palace/manager/core/config.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/core/config.py#L75

Added line #L75 was not covered by tests
if not url:
raise CannotLoadConfiguration(
"Database URL was not defined in environment variable (%s)."
% environment_variable
% cls.DATABASE_PRODUCTION_ENVIRONMENT_VARIABLE
)

url_obj = None
try:
url_obj = make_url(url)
except ArgumentError as e:
# Improve the error message by giving a guide as to what's
# likely to work.
# Improve the error message by giving a guide as to what's likely to work.
raise ArgumentError(
"Bad format for database URL (%s). Expected something like postgresql://[username]:[password]@[hostname]:[port]/[database name]"
% url
)

# Calling __to_string__ will hide the password.
logging.info("Connecting to database: %s" % url_obj.__to_string__())
logging.info(

Check warning on line 91 in src/palace/manager/core/config.py

View check run for this annotation

Codecov / codecov/patch

src/palace/manager/core/config.py#L91

Added line #L91 was not covered by tests
"Connecting to database: %s" % url_obj.render_as_string(hide_password=True)
)
return url

@classmethod
Expand Down
5 changes: 4 additions & 1 deletion src/palace/manager/scripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,10 @@ class InstanceInitializationScript:
remain idempotent.
"""

def __init__(self, config_file: Path | None = None) -> None:
def __init__(
self,
config_file: Path | None = None,
) -> None:
self._log: logging.Logger | None = None
self._container = container_instance()

Expand Down
15 changes: 2 additions & 13 deletions src/palace/manager/sqlalchemy/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import json
import logging
import os
from typing import Any

from pydantic.json import pydantic_encoder
Expand Down Expand Up @@ -62,18 +61,8 @@ def setup_event_listener(
return session

@classmethod
def sessionmaker(cls, url=None, session=None):
if not (url or session):
url = Configuration.database_url()
if url:
bind_obj = cls.engine(url)
elif session:
bind_obj = session.get_bind()
if not os.environ.get("TESTING"):
# If a factory is being created from a session in test mode,
# use the same Connection for all of the tests so objects can
# be accessed. Otherwise, bind against an Engine object.
bind_obj = bind_obj.engine
def sessionmaker(cls):
bind_obj = cls.engine()
session_factory = sessionmaker(bind=bind_obj)
cls.setup_event_listener(session_factory)
return session_factory
Expand Down
26 changes: 6 additions & 20 deletions tests/fixtures/api_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def __init__(
self,
db: DatabaseTransactionFixture,
services_fixture: ServicesFixture,
setup_cm: bool,
):
self.db = db
self.app = app
Expand All @@ -102,11 +101,10 @@ def __init__(
self.search_index = ExternalSearchIndexFake()
self.services_fixture.services.search.index.override(self.search_index)

if setup_cm:
# NOTE: Any reference to self._default_library below this
# point in this method will cause the tests in
# TestScopedSession to hang.
app.manager = self.circulation_manager_setup()
# NOTE: Any reference to self._default_library below this
# point in this method will cause the tests in
# TestScopedSession to hang.
app.manager = self.circulation_manager_setup()

@contextmanager
def wired_container(self):
Expand Down Expand Up @@ -259,19 +257,7 @@ def controller_fixture(
db: DatabaseTransactionFixture, services_fixture: ServicesFixture
):
time_then = datetime.datetime.now()
fixture = ControllerFixture(db, services_fixture, setup_cm=True)
time_now = datetime.datetime.now()
time_diff = time_now - time_then
logging.info("controller init took %s", time_diff)
yield fixture


@pytest.fixture(scope="function")
def controller_fixture_without_cm(
db: DatabaseTransactionFixture, services_fixture: ServicesFixture
):
time_then = datetime.datetime.now()
fixture = ControllerFixture(db, services_fixture, setup_cm=False)
fixture = ControllerFixture(db, services_fixture)
time_now = datetime.datetime.now()
time_diff = time_now - time_then
logging.info("controller init took %s", time_diff)
Expand Down Expand Up @@ -315,7 +301,7 @@ class CirculationControllerFixture(ControllerFixture):
def __init__(
self, db: DatabaseTransactionFixture, services_fixture: ServicesFixture
):
super().__init__(db, services_fixture, setup_cm=True)
super().__init__(db, services_fixture)
self.works = []
self.add_works(self.BOOKS)

Expand Down
Loading

0 comments on commit 9488ba1

Please sign in to comment.