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 3, 2024
1 parent 81c439c commit 182efbc
Show file tree
Hide file tree
Showing 40 changed files with 946 additions and 723 deletions.
30 changes: 28 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ jobs:
bash -c "
source env/bin/activate &&
poetry install --without ci --no-root --sync &&
pytest --no-cov tests/manager
pytest --no-cov tests
"
env:
BUILD_CACHE_FROM: type=gha,scope=buildkit-${{ github.run_id }}
Expand All @@ -205,10 +205,36 @@ jobs:
if: always()
run: docker compose down

migration-test:
name: Migration test
runs-on: ubuntu-latest
needs: [build]
permissions:
contents: read

steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
fetch-depth: 0

# See comment here: https://github.com/actions/runner-images/issues/1187#issuecomment-686735760
- name: Disable network offload
run: sudo ethtool -K eth0 tx off rx off

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

- name: Test migrations
run: ./docker/ci/test_migrations.sh
env:
BUILD_CACHE_FROM: type=gha,scope=buildkit-${{ github.run_id }}
BUILD_BASE_IMAGE: ${{ needs.build.outputs.baseimage }}

push:
name: Push circ-${{ matrix.image }}
runs-on: ubuntu-latest
needs: [build, integration-test, unit-test]
needs: [build, integration-test, unit-test, migration-test]
permissions:
contents: read
packages: write
Expand Down
66 changes: 1 addition & 65 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,69 +52,5 @@ jobs:
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./coverage.xml
flags: manager
name: manager-${{ matrix.python-version }}
name: test-${{ matrix.python-version }}
verbose: true

test-migrations:
name: Migration Tests
runs-on: ubuntu-latest
permissions:
contents: read

steps:
- uses: actions/checkout@v4

# See comment here: https://github.com/actions/runner-images/issues/1187#issuecomment-686735760
- name: Disable network offload
run: sudo ethtool -K eth0 tx off rx off

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.10"

- name: Install Poetry
uses: ./.github/actions/poetry
with:
cache: true

- name: Install Tox
run: |
poetry install --only ci --no-root
env:
POETRY_VIRTUALENVS_CREATE: false

- name: Run Migration Tests
run: tox -e "migration-docker"

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
files: ./coverage.xml
flags: migration
name: "migration-3.10"
verbose: true

docker-test-migrations:
name: Docker migration test
runs-on: ubuntu-latest
permissions:
contents: read

steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
fetch-depth: 0

# See comment here: https://github.com/actions/runner-images/issues/1187#issuecomment-686735760
- name: Disable network offload
run: sudo ethtool -K eth0 tx off rx off

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3

- name: Test migrations
run: ./docker/ci/test_migrations.sh
23 changes: 12 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -636,20 +636,27 @@ tox -e "py310-docker"
If you already have elastic search or postgres running locally, you can run them instead by setting the
following environment variables:

- `SIMPLIFIED_TEST_DATABASE`
- `PALACE_TEST_DATABASE_URL`
- `PALACE_TEST_SEARCH_URL`

Make sure the ports and usernames are updated to reflect the local configuration.

```sh
# Set environment variables
export SIMPLIFIED_TEST_DATABASE="postgresql://simplified_test:test@localhost:9005/simplified_circulation_test"
export SIMPLIFIED_TEST_OPENSEARCH="http://localhost:9200"
export PALACE_TEST_DATABASE_URL="postgresql://simplified_test:test@localhost:9005/simplified_circulation_test"
export PALACE_TEST_SEARCH_URL="http://localhost:9200"

# Run tox
tox -e "py310"
```

The tests assume that they have permission to create and drop databases. They connect to the
provided database URL and create a new database for each test run. If the user does not have permission
to create and drop databases, the tests will fail. You can disable this behavior by setting the
`PALACE_TEST_DATABASE_CREATE_DATABASE` environment variable to `false`.

```sh

### Override `pytest` arguments

If you wish to pass additional arguments to `pytest` you can do so through `tox`. Every argument passed after a `--` to
Expand Down Expand Up @@ -752,16 +759,10 @@ This profiler uses [PyInstrument](https://pyinstrument.readthedocs.io/en/latest/
PyInstrument can also be used to profile the test suite. This can be useful to identify slow tests, or to identify
performance regressions.
To profile the core test suite, run the following command:

```sh
pyinstrument -m pytest --no-cov tests/core/
```

To profile the API test suite, run the following command:
To profile the test suite, run the following command:
```sh
pyinstrument -m pytest --no-cov tests/api/
pyinstrument -m pytest --no-cov -n 0 tests
```
#### Environment Variables
Expand Down
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
2 changes: 1 addition & 1 deletion docker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ Environment variables can be set with the `-e VARIABLE_KEY='variable_value'` opt

*Required.* The URL of the production PostgreSQL database for the application.

### `SIMPLIFIED_TEST_DATABASE`
### `PALACE_TEST_DATABASE_URL`

*Optional.* The URL of a PostgreSQL database for tests. This optional variable allows unit tests to be run in the
container.
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=worksteal",
"--numprocesses=auto",
"--strict-markers",
]
markers = [
Expand Down
34 changes: 14 additions & 20 deletions src/palace/manager/api/app.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import logging
import os

import flask_babel
from flask import request
Expand All @@ -17,7 +16,7 @@
PalaceXrayProfiler,
)
from palace.manager.core.app_server import ErrorHandler
from palace.manager.service.container import Services, container_instance
from palace.manager.service.container import container_instance
from palace.manager.sqlalchemy.flask_sqlalchemy_session import flask_scoped_session
from palace.manager.sqlalchemy.model.key import Key, KeyType
from palace.manager.sqlalchemy.session import SessionManager
Expand Down Expand Up @@ -66,24 +65,19 @@ def initialize_admin(_db: Session | None = None):
).value


def initialize_circulation_manager(container: Services):
if os.environ.get("AUTOINITIALIZE") == "False":
# It's the responsibility of the importing code to set app.manager
# appropriately.
pass
else:
if getattr(app, "manager", None) is None:
try:
app.manager = CirculationManager(app._db)
except Exception:
logging.exception("Error instantiating circulation manager!")
raise
# Make sure that any changes to the database (as might happen
# on initial setup) are committed before continuing.
app.manager._db.commit()
def initialize_circulation_manager():
if getattr(app, "manager", None) is None:
try:
app.manager = CirculationManager(app._db)
except Exception:
logging.exception("Error instantiating circulation manager!")
raise
# Make sure that any changes to the database (as might happen
# on initial setup) are committed before continuing.
app.manager._db.commit()

# setup the cache data object
CachedData.initialize(app._db)
# setup the cache data object
CachedData.initialize(app._db)


def initialize_database():
Expand Down Expand Up @@ -113,6 +107,6 @@ def initialize_application() -> PalaceFlask:
app.register_error_handler(Exception, error_handler.handle)

# Initialize the circulation manager
initialize_circulation_manager(container)
initialize_circulation_manager()
initialize_admin()
return app
Loading

0 comments on commit 182efbc

Please sign in to comment.