From 4ad80c3270456fd69174cfd173ea7ed5d1f05ad2 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Tue, 10 Jan 2023 17:09:37 -0500 Subject: [PATCH 1/6] Run tests outside Docker --- .github/workflows/test-build.yaml | 26 +++++++++----------------- Makefile | 7 ++++--- bin/test.sh | 15 +++++++++++++++ docker-compose.yaml | 20 ++------------------ docker/test.sh | 10 ---------- 5 files changed, 30 insertions(+), 48 deletions(-) create mode 100755 bin/test.sh delete mode 100755 docker/test.sh diff --git a/.github/workflows/test-build.yaml b/.github/workflows/test-build.yaml index 61d432ee..07c6f403 100644 --- a/.github/workflows/test-build.yaml +++ b/.github/workflows/test-build.yaml @@ -23,25 +23,17 @@ jobs: --health-retries 5 steps: - - name: Checkout code - uses: actions/checkout@v2 - - - name: Build test image - uses: docker/build-push-action@v2 + - uses: actions/checkout@v3 + - name: Install poetry + run: pipx install poetry + - uses: actions/setup-python@v4 with: - context: . - file: docker/Dockerfile - push: false - target: "development" - tags: ghcr.io/${{ github.repository }}:${{ github.sha }} - + python-version: "3.10" + cache: "poetry" + - name: Install dependencies + run: poetry install - name: Run tests - run: |- - docker run --rm \ - -e CTMS_DB_URL \ - -e CTMS_SECRET_KEY \ - ghcr.io/${{ github.repository }}:${{ github.sha }} \ - ./docker/test.sh + run: bin/test.sh env: CTMS_DB_URL: postgresql://postgres:postgres@172.17.0.1:5432/postgres CTMS_SECRET_KEY: secret_${{ github.sha }} diff --git a/Makefile b/Makefile index ff3b34fa..35d1bb79 100644 --- a/Makefile +++ b/Makefile @@ -72,9 +72,10 @@ start: .env docker-compose up .PHONY: test -test: .env - docker-compose run --rm ${MK_WITH_SERVICE_PORTS} tests -ifneq (1, ${MK_KEEP_DOCKER_UP}) +test: .env $(INSTALL_STAMP) + docker-compose up --wait postgres + bin/test.sh +ifneq (1, ${MK_KEEP_DOCKER_UP}) # Due to https://github.com/docker/compose/issues/2791 we have to explicitly # rm all running containers docker-compose down diff --git a/bin/test.sh b/bin/test.sh new file mode 100755 index 00000000..e53150be --- /dev/null +++ b/bin/test.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +set -euo pipefail + +POETRY_RUN="poetry run" + +CURRENT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +BASE_DIR="$(dirname "$CURRENT_DIR")" + +# without this, some tests fail because of off-by-timezone errors. +export TZ=UTC + +$POETRY_RUN coverage run --rcfile "${BASE_DIR}/pyproject.toml" -m pytest +$POETRY_RUN coverage report --rcfile "${BASE_DIR}/pyproject.toml" -m --fail-under 80 +$POETRY_RUN coverage html --rcfile "${BASE_DIR}/pyproject.toml" diff --git a/docker-compose.yaml b/docker-compose.yaml index 07c2b937..2e495311 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -4,12 +4,9 @@ services: build: context: . dockerfile: docker/Dockerfile - target: development image: web - volumes: - - type: bind - source: . - target: /app + volumes: + - .:/app ports: - ${PORT:-8000}:${PORT:-8000} # Let the init system handle signals for us. @@ -43,16 +40,3 @@ services: ports: - 8080:8080 - tests: - image: web - depends_on: - web: - condition: service_started - postgres: - condition: service_healthy - command: ["bash", "./docker/test.sh"] - profiles: [test] - init: true - env_file: - - docker/config/local_dev.env - - .env diff --git a/docker/test.sh b/docker/test.sh deleted file mode 100755 index 15812753..00000000 --- a/docker/test.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -set -euo pipefail - -CURRENT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) -BASE_DIR="$(dirname "$CURRENT_DIR")" - -coverage run --rcfile "${BASE_DIR}/pyproject.toml" -m pytest -coverage report --rcfile "${BASE_DIR}/pyproject.toml" -m --fail-under 80 -coverage html --rcfile "${BASE_DIR}/pyproject.toml" From 31e3f4abe1a8af4329056a9c9ec98a4fdbd6a578 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Tue, 10 Jan 2023 18:07:38 -0500 Subject: [PATCH 2/6] Simplify Dockerfile Now that we run tests outside of the Docker container, we now only need two stages in our multistage build: - one stage to build Python dependencies - another stage to copy in the dependencies and run the application --- bin/update_and_install_system_packages.sh | 14 ++++ docker/Dockerfile | 97 ++++++----------------- 2 files changed, 39 insertions(+), 72 deletions(-) create mode 100755 bin/update_and_install_system_packages.sh diff --git a/bin/update_and_install_system_packages.sh b/bin/update_and_install_system_packages.sh new file mode 100755 index 00000000..4a9278e6 --- /dev/null +++ b/bin/update_and_install_system_packages.sh @@ -0,0 +1,14 @@ +#!/bin/bash +set -euo pipefail +# Tell apt-get we're never going to be able to give manual +# feedback: +export DEBIAN_FRONTEND=noninteractive + +apt-get update +# Install security updates +apt-get -y upgrade +# Install packages +apt-get -y install --no-install-recommends $@ +# Delete cached files we don't need anymore +apt-get clean +rm -rf /var/lib/apt/lists/* diff --git a/docker/Dockerfile b/docker/Dockerfile index 05035337..bfe29694 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,91 +1,44 @@ -# Dockerfile -# Uses multi-stage builds requiring Docker 17.05 or higher -# See https://docs.docker.com/develop/develop-images/multistage-build/ +FROM python:3.10.6 as python-base -# Creating a python base with shared environment variables -FROM python:3.10.6-slim as python-base -ENV PYTHONPATH=/app \ - PYTHONUNBUFFERED=1 \ - PYTHONDONTWRITEBYTECODE=1 \ - PIP_NO_CACHE_DIR=off \ +ENV PIP_DEFAULT_TIMEOUT=100 \ PIP_DISABLE_PIP_VERSION_CHECK=on \ - PIP_DEFAULT_TIMEOUT=100 \ - POETRY_HOME="/opt/poetry" \ - POETRY_VIRTUALENVS_IN_PROJECT=true \ + PIP_NO_CACHE_DIR=off \ + POETRY_HOME=/opt/poetry\ POETRY_NO_INTERACTION=1 \ - PYSETUP_PATH="/opt/pysetup" \ - VENV_PATH="/opt/pysetup/.venv" \ - PORT=8000 - -ENV PATH="$VENV_PATH/bin:$POETRY_HOME/bin:$PATH" - -# Set up user and group -ARG userid=10001 -ARG groupid=10001 -WORKDIR /app -RUN groupadd --gid $groupid app && \ - useradd -g app --uid $userid --shell /usr/sbin/nologin --create-home app - -RUN mkdir -p $POETRY_HOME && \ - chown app:app /opt/poetry && \ - mkdir -p $PYSETUP_PATH && \ - chown app:app $PYSETUP_PATH && \ - mkdir -p /app && \ - chown app:app /app - -RUN apt-get update && \ - apt-get install --assume-yes apt-utils && \ - echo 'debconf debconf/frontend select Noninteractive' | debconf-set-selections && \ - apt-get install --no-install-recommends -y \ - libpq5 - -# builder-base is used to build dependencies -FROM python-base as builder-base -RUN apt-get install --no-install-recommends -y \ - build-essential \ - libpq-dev + POETRY_VIRTUALENVS_IN_PROJECT=true \ + PYSETUP_PATH="/opt/pysetup" -# Install Poetry - respects $POETRY_VERSION & $POETRY_HOME -USER app RUN python3 -m venv $POETRY_HOME && \ $POETRY_HOME/bin/pip install poetry==1.3.1 && \ $POETRY_HOME/bin/poetry --version -# We copy our Python requirements here to cache them -# and install only runtime deps using poetry WORKDIR $PYSETUP_PATH -COPY --chown=app:app ./poetry.lock ./pyproject.toml ./ -RUN poetry install --no-dev --no-root +COPY ./poetry.lock ./pyproject.toml ./ +RUN $POETRY_HOME/bin/poetry install --no-dev --no-root +FROM python:3.10.6-slim as production -# 'development' stage installs all dev deps and can be used to develop code. -# For example using docker-compose to mount local volume under /app -FROM python-base as development -ENV FASTAPI_ENV=development +COPY bin/update_and_install_system_packages.sh /opt +RUN opt/update_and_install_system_packages.sh libpq5 -# Copying poetry and venv into image -USER app -COPY --from=builder-base $POETRY_HOME $POETRY_HOME -COPY --from=builder-base $PYSETUP_PATH $PYSETUP_PATH +ENV PATH="/opt/pysetup/.venv/bin:$PATH" \ + PORT=8000 \ + PYTHONDONTWRITEBYTECODE=1 \ + PYTHONFAULTHANDLER=1 \ + PYTHONUNBUFFERED=1 \ + PYTHONPATH=/app \ + VENV_PATH="/opt/pysetup/.venv" -# venv already has runtime deps installed we get a quicker install -WORKDIR $PYSETUP_PATH -RUN poetry install --no-root +COPY --from=python-base $VENV_PATH $VENV_PATH +# Set up user and group +ARG userid=10001 +ARG groupid=10001 +RUN groupadd --gid $groupid app && \ + useradd -g app --uid $userid --shell /usr/sbin/nologin --create-home app +USER app WORKDIR /app -COPY --chown=app:app . . - -EXPOSE $PORT -CMD ["python", "asgi.py"] - -# 'production' stage uses the clean 'python-base' stage and copyies -# in only our runtime deps that were installed in the 'builder-base' -FROM python-base as production -ENV FASTAPI_ENV=production -COPY --from=builder-base $VENV_PATH $VENV_PATH - -WORKDIR /app COPY --chown=app:app . . EXPOSE $PORT From bc5299ccebb4cdf3c94a47f20b9daf88c4dcfb44 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Wed, 11 Jan 2023 11:04:32 -0500 Subject: [PATCH 3/6] Move update_baseline to `bin/` directory --- bin/update_baseline.sh | 7 +++++++ scripts/update_baseline.sh | 8 -------- 2 files changed, 7 insertions(+), 8 deletions(-) create mode 100755 bin/update_baseline.sh delete mode 100755 scripts/update_baseline.sh diff --git a/bin/update_baseline.sh b/bin/update_baseline.sh new file mode 100755 index 00000000..f32a12e5 --- /dev/null +++ b/bin/update_baseline.sh @@ -0,0 +1,7 @@ +#!/bin/bash + +set -euo pipefail + +detect-secrets scan . \ +--baseline .secrets.baseline \ +--exclude-files "(poetry.lock$)|(htmlcov/)" diff --git a/scripts/update_baseline.sh b/scripts/update_baseline.sh deleted file mode 100755 index 05442378..00000000 --- a/scripts/update_baseline.sh +++ /dev/null @@ -1,8 +0,0 @@ -#!/bin/bash - -set -e - -CURRENT_DIR=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) -BASE_DIR="$(dirname "$CURRENT_DIR")" - -detect-secrets scan . --exclude-files "(poetry.lock$)|(htmlcov/)" > .secrets.baseline From 104755794f7ff07fa6f5739362ee64cd9b65a22c Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Wed, 11 Jan 2023 11:05:25 -0500 Subject: [PATCH 4/6] Add `make` rule for update-secrets Also, alpha sort make rules --- Makefile | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 35d1bb79..0fdf574c 100644 --- a/Makefile +++ b/Makefile @@ -17,14 +17,14 @@ help: @echo "" @echo "CTMS make rules:" @echo "" - @echo " build - build docker containers" - @echo " lint - lint check for code" - @echo " setup - (re)create the database" - @echo " start - run the API service" - @echo "" - @echo " test - run test suite" - @echo " shell - open a shell in the web container" - @echo " db-only - run PostgreSQL server" + @echo " build - build docker containers" + @echo " db-only - run PostgreSQL server" + @echo " lint - lint check for code" + @echo " setup - (re)create the database" + @echo " shell - open a shell in the web container" + @echo " start - run the API service" + @echo " test - run test suite" + @echo " update-secrets - scan repository for secrets and update baseline file, if necessary" @echo "" @echo " help - see this text" @@ -81,3 +81,6 @@ ifneq (1, ${MK_KEEP_DOCKER_UP}) docker-compose down endif +.PHONY: update-secrets +update-secrets: + bin/update_baseline.sh From 4da2b427d18f579382a252dec9419ec5df79ce8d Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Wed, 11 Jan 2023 11:08:20 -0500 Subject: [PATCH 5/6] Update documentation with new testing procedures --- README.md | 4 ++-- docs/developer_setup.md | 9 ++------ docs/testing_strategy.md | 45 ++++++++++++++++++++++------------------ 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index a00312f3..7c84ee31 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,8 @@ for this project to ensure correct environment setup. The project is structured with the following in mind: +- [bin/*](bin/) + - Some scripts that have proven useful within the CTMS ecosystem - [docs/*](docs/) - Documentation to guide others around the project interactions - [ctms/*](ctms/) @@ -30,8 +32,6 @@ The project is structured with the following in mind: - Pydantic Models for Data Modeling and Contract Validation - [migrations/*](migrations/) - Alembic migrations that act as a changelog or version control system for implementing DB changes in an ordered fashion -- [scripts/*](scripts/) - - Some scripts that have proven useful within the CTMS ecosystem - [tests/unit/*](test/unit/) - Test suite using pytest diff --git a/docs/developer_setup.md b/docs/developer_setup.md index 6f7bdd21..2679d967 100644 --- a/docs/developer_setup.md +++ b/docs/developer_setup.md @@ -20,15 +20,10 @@ curl -sSL https://install.python-poetry.org | python3 - ``` ## Setup Dependencies -Install prod dependencies in pyproject.toml through poetry: +To set up dependencies necessary for local development, run: ```sh -poetry install --no-dev -``` - -Install ALL deps including DEV dependencies: -```sh -poetry install +make install ``` If these fail with an error like: diff --git a/docs/testing_strategy.md b/docs/testing_strategy.md index 97f2d290..f2bd7c86 100644 --- a/docs/testing_strategy.md +++ b/docs/testing_strategy.md @@ -1,44 +1,49 @@ # Testing Strategy -We strive for high test coverage, and to add tests for new functionality and +We strive for high test coverage and to add tests for new functionality and bug fixes that are merged with the code changes. ## Running the tests -Tests require a database, which is trivial with ``docker-compose``. -All tests can be run from the command: +All tests can be run with the command: > make test -This starts the database container, enters the web container, runs the unit -tests, and calculates the combined code coverage. See -``scripts/test.sh`` for details. - -During development, you may want to run a subset of tests. You can enter the -web container with ``make test-shell``, and then run the test commands directly. -See the sections below for details. +This starts a database container, waits for it to be ready, runs the unit +tests, and calculates the combined code coverage. See ``bin/test.sh`` for +details. ## Pytest for unit tests -We are using [pytest][pytest] for unit testing, as well as integration testing +We are using [pytest][pytest] for unit testing as well as integration testing with the database. -- The tests live in: tests/unit/*.py -- The shared test fixtures live in: tests/unit/conftest.py - -[pytest]: "pytest documentation" +- The tests live in: `tests/unit/*.py` +- The shared test fixtures live in: `tests/unit/conftest.py` -### To run the tests: -You can run the suite by first entering the web container: -> make shell +### Running tests manually: +Make sure you have dependencies installed and a postgres database running with +all of the migrations run And then run the installed ``pytest``: -> pytest +```sh +pytest +``` To stop on the first failure and drop into [pdb][pdb]: -> pytest -sx --pdb +```sh +pytest -sx --pdb +``` + +To run a test or tests whose name matchs a substring: +```sh +pytest -k "substring" +``` + +View the [pytest] documentation for more options. [pdb]: "pdb - The Python Debugger" +[pytest]: "pytest documentation" --- [View All Docs](./) From be588638e68af4562cfddca1547f618a33b304e8 Mon Sep 17 00:00:00 2001 From: Graham Beckley Date: Wed, 11 Jan 2023 11:08:36 -0500 Subject: [PATCH 6/6] Update .secrets.baseline --- .secrets.baseline | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 00b7a3d9..f36ffbae 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -131,14 +131,14 @@ "filename": ".github/workflows/test-build.yaml", "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", "is_verified": false, - "line_number": 46 + "line_number": 38 }, { "type": "Secret Keyword", "filename": ".github/workflows/test-build.yaml", "hashed_secret": "a2490b1a07aa4a72606afe91f2de20f8c524d779", "is_verified": false, - "line_number": 47 + "line_number": 39 } ], "bin/lint.sh": [ @@ -165,7 +165,7 @@ "filename": "docs/developer_setup.md", "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3", "is_verified": false, - "line_number": 208 + "line_number": 203 } ], "tests/unit/test_acoustic_service.py": [ @@ -203,5 +203,5 @@ } ] }, - "generated_at": "2022-11-29T20:27:29Z" + "generated_at": "2023-01-11T16:11:56Z" }