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

Browser-tests and integration-tests merge, paving trail for better gatekeeper tests #566

Merged
merged 12 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
53 changes: 4 additions & 49 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -93,51 +93,6 @@ jobs:
run: make install
- name: Run tests
run: make test
integration_test:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Everything runs under browser-tests now

runs-on: ubuntu-latest
env:
DOCKER_CACHE: /tmp/docker-cache
steps:
- uses: actions/checkout@v3

- run: mkdir ${DOCKER_CACHE}

- name: Compute cache key
# Create hash of hashes of checked in files not in Dockerignore
run: echo "CACHE_KEY=$(git ls-tree --full-tree -r HEAD | grep -v -f RemoteSettings.dockerignore | awk '{print $3}' | git hash-object --stdin)" >> $GITHUB_ENV

- uses: actions/cache@v3
with:
path: ${{ env.DOCKER_CACHE}}
key: docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-${{ env.CACHE_KEY }}
restore-keys: |
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-${{ env.CACHE_KEY }}
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-
docker-build-

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

- name: Build web container
uses: docker/build-push-action@v5
with:
cache-from: type=local,src=${{ env.DOCKER_CACHE}}/web
cache-to: type=local,dest=${{ env.DOCKER_CACHE}}/web,mode=max
file: RemoteSettings.Dockerfile
context: .

- name: Build test container
uses: docker/build-push-action@v5
with:
cache-from: type=local,src=${{ env.DOCKER_CACHE}}/tests
cache-to: type=local,dest=${{ env.DOCKER_CACHE}}/tests,mode=max
file: IntegrationTests.Dockerfile
context: .

- name: Run integration tests
run: make integration-test

browser-test:
runs-on: ubuntu-latest
Expand All @@ -153,10 +108,10 @@ jobs:
- uses: actions/cache@v3
with:
path: ${{ env.DOCKER_CACHE}}
key: docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-${{ env.CACHE_KEY }}
key: docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'BrowserTests.Dockerfile') }}-${{ env.CACHE_KEY }}
restore-keys: |
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-${{ env.CACHE_KEY }}
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'IntegrationTests.Dockerfile') }}-
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'BrowserTests.Dockerfile') }}-${{ env.CACHE_KEY }}
docker-build-${{ hashFiles('RemoteSettings.Dockerfile', 'BrowserTests.Dockerfile') }}-
docker-build-

- name: Set up Docker Buildx
Expand All @@ -177,7 +132,7 @@ jobs:
with:
cache-from: type=local,src=${{ env.DOCKER_CACHE}}/tests
cache-to: type=local,dest=${{ env.DOCKER_CACHE}}/tests,mode=max
file: IntegrationTests.Dockerfile
file: BrowserTests.Dockerfile
context: .

- name: Run browser tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are the logs so long for this step? Could we make sure we don't have to scroll that much in the workflows step logs before seeing the tests results?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're the same warnings as before I think. See this previous integration-tests run and this previous browser-tests run. Five DeprecationWarnings from autograph_utils, one from kinto_http, and a config warning about sensitive_url.

I could suppress the warnings but that seems like a bad idea. We should probably get updating autograph and kinto_http on the radar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added -q to docker compose build to quiet that part down at least.

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ jobs:
push: true
tags: ${{ steps.docker-metadata.outputs.tags }}

integration_test_container:
browser_test_container:
runs-on: ubuntu-latest
steps:
- name: Checkout code
Expand All @@ -62,7 +62,7 @@ jobs:
id: docker-metadata
uses: docker/metadata-action@v5
with:
images: mozilla/remote-settings-integration-tests
images: mozilla/remote-settings-browser-tests
tags: |
type=raw,value=${{ env.LATEST_TAG }}
type=raw,value=latest
Expand All @@ -75,6 +75,6 @@ jobs:
uses: docker/build-push-action@v5
with:
context: .
file: IntegrationTests.Dockerfile
file: BrowserTests.Dockerfile
push: true
tags: ${{ steps.docker-metadata.outputs.tags }}
2 changes: 1 addition & 1 deletion IntegrationTests.Dockerfile → BrowserTests.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ RUN python -m venv $POETRY_HOME && \

WORKDIR /opt
COPY pyproject.toml poetry.lock ./
RUN $POETRY_HOME/bin/poetry install --only integration-tests --no-root
RUN $POETRY_HOME/bin/poetry install --only browser-tests --no-root

FROM python:3.12.2

Expand Down
File renamed without changes.
7 changes: 1 addition & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -47,19 +47,14 @@ test: $(INSTALL_STAMP) ## Run unit tests
PYTHONPATH=. $(VENV)/bin/coverage run -m pytest kinto-remote-settings
$(VENV)/bin/coverage report -m --fail-under 99

integration-test: ## Run integration tests using Docker
docker compose build tests
docker compose run --rm web migrate
docker compose run --rm tests integration-test

browser-test: ## Run browser tests using Docker
docker compose build tests
docker compose run --rm web migrate
docker compose run --rm tests browser-test

build: ## Build containers
docker build --file RemoteSettings.Dockerfile --target production --tag remotesettings/server .
docker compose --profile integration-test build
docker compose --profile browser-test build

build-db: ## Initialize database 'postgresql://postgres@localhost/testdb'
ifdef PSQL_INSTALLED
Expand Down
41 changes: 15 additions & 26 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ This *Remote Settings* repository contains the following files and directories o
* ``config/``: example configuration file(s)
* ``docs/``: documentation source files
* ``kinto-remote-settings/``: Kinto plugin specific to Remote Settings
* ``tests/``: browser and integration tests
* ``tests/``: browser/integration/gatekeeper tests
* ``pyproject.toml``: contains dependency information and (most) config settings
* ``VERSION``: SemVer version number that serves as both the version of the service and the ``kinto-remote-settings`` plugin

Expand Down Expand Up @@ -59,14 +59,13 @@ After this setup is complete, tests can be run with ``pytest`` using ``make``:
make test


**Integration & Browser Tests**
**Browser Tests**

With Docker and docker-compose, test that all components are working as expected with:

.. code-block:: shell

make build
make integration-test
make browser-test

.. note::
Expand All @@ -92,15 +91,15 @@ finished, run:
Test Remote Server
------------------

Integration tests can be executed on a remote server or against the docker-compose containers.
Browser tests can be executed on a remote server or against the docker-compose containers.

To run the integration test suite, first build the integration tests container
To run the test suite, first build the tests container

.. code-block:: shell

docker-compose build tests

or download a pre-built container from `Dockerhub <https://hub.docker.com/r/mozilla/remote-settings-integration-tests>`_.
or download a pre-built container from `Dockerhub <https://hub.docker.com/r/mozilla/remote-settings-browser-tests>`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note to delete the mozilla/remote-settings-integration-tests image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify - do you mean add a note in here telling people to delete their old downloaded images? Or add a note for us to remove the old images from dockerhub?

Copy link
Contributor

Choose a reason for hiding this comment

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

for us to remove the old images from dockerhub


Next run the tests, supplying config values as necessary. Config values are
set as environment variables provided to the Docker container. See
Expand All @@ -111,45 +110,36 @@ Note that the tests assume that the server has the ``attachments``,
``changes``, ``history``, and ``signer`` plugins enabled. It may optionally
have the ``email`` plugin installed.

To have the tests bootstrap themselves (i.e. when ``SKIP_SERVER_SETUP=false``),
the credentials passed in ``SETUP_AUTH`` should have the permission to create
users, buckets, and collections. These credentials will be in the form
The credentials passed in ``SETUP_AUTH`` should have the permission to create users,
buckets, and collections. These credentials will be in the form
``SETUP_AUTH=username:password`` or ``SETUP_AUTH="Bearer some_token"``

If the tests should not bootstrap themselves and instead use resources already
available on the server (i.e. when ``SKIP_SERVER_SETUP=true``):

- There should be a bucket and collection available

- the bucket, if not specified by the ``BUCKET`` config option, should be named ``main-workspace``
- the collection, if not specified by the ``COLLECTION`` config option, should be named ``integration-tests``

- All tests will run under the `browser-tests` collection in the `main-workspace` bucket
leplatrem marked this conversation as resolved.
Show resolved Hide resolved
- If the collection does not exist it will be created
- There should be two users available

- one user should be added to the ``editors`` group of the available collection
- the other should be added to the ``reviewers`` group of the available collection
- the credentials of these users should be passed in the ``EDITOR_AUTH`` and
``REVIEWER_AUTH`` config options respectively

Running integration tests on the Remote Settings DEV server should look something like:
Running browser tests on the Remote Settings DEV server should look something like:

.. code-block:: shell

docker run --rm \
--env SERVER=https://remote-settings-dev.allizom.org/v1 \
--env MAIL_DIR="" `#disables test cases related to emails` \
--env SKIP_SERVER_SETUP=true \
--env TO_REVIEW_ENABLED=false \
--env EDITOR_AUTH=<username:password, credentials available in 1Password> \
--env REVIEWER_AUTH=<username:password, available in 1Password> \
remotesettings/tests integration-test
remotesettings/tests browser-test


Because the integration tests are capable of running against environments with existing data, there are limitations to what they can do. Examples:
- Test server setup is global and may be skipped entirely against an existing server
Because the tests are capable of running against environments with existing data, there are limitations to what they can do. Examples:
- Test setup is global
- Test setup and may be partially skipped if the bucket, collection and users already exist
- All tests have access to the same bucket, collection, and users
- Tests are not allowed to delete the bucket(s), collection(s) or users
- Test records may not be purged if the remote server disables this ability
- Test collection records are purged before each test



Expand All @@ -160,7 +150,6 @@ The simplest form of debugging is to run a suite of tests against the Kinto serv

.. code-block:: shell

make integration-test
make browser-test

Debugging Locally (advanced)
Expand Down
2 changes: 1 addition & 1 deletion RemoteSettings.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,5 @@ ENTRYPOINT ["./bin/run.sh"]
CMD ["start"]

FROM production as local
# create directories for volume mounts used in integration tests / local development
# create directories for volume mounts used in browser tests / local development
RUN mkdir -p -m 777 /app/mail && mkdir -p -m 777 /tmp/attachments
4 changes: 2 additions & 2 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ services:

tests:
build:
dockerfile: IntegrationTests.Dockerfile
dockerfile: BrowserTests.Dockerfile
context: .
image: remotesettings/tests
profiles: [integration-test]
profiles: [browser-test]
depends_on:
- web
- certchains
Expand Down
Loading
Loading