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

Add docker compose based test environment for Linux #687

Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ shippable/
# htmlcov is generated by tox due to coverage gathering in pytest
htmlcov/
.dir-locals.el
docker-testing/outdir
28 changes: 21 additions & 7 deletions MAINTAINERS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,34 @@ Pre-release test plan

git checkout vX.Y-branch

1. Make tox happy on the following first-party platforms:
1. Make tox happy on the following first-party non-Linux platforms:

- Windows 10
- the latest macOS
- the latest Ubuntu LTS

2. Make tox happy on other popular Linux distributions as resources allow.
Doing this in a container is fine.
Do this by hand and check for any anomalous warnings in the output.
Do not just trust CI.

2. Make tox happy on other popular Linux distributions:

- Arch
- the latest Ubuntu release (if different than the latest LTS)
- Debian stable (if its Python 3 is still supported)
- the latest Ubuntu LTS (22.04 at time of writing)
- the latest Ubuntu release (23.04 at time of writing)
- Debian stable (12 at time of writing)
- Debian testing
- Fedora
- the latest Fedora (38 at time of writing)

Automated infrastructure for doing this in docker is in the docker-testing
directory. Start by updating the Dockerfiles and compose.yaml in that
directory if any newer distribution versions should be tested.

Then, install docker compose in your host Linux environment and run::

cd docker-testing
./run-tests.sh

Make sure to check the tox.log files mentioned in the output for any
anomalous warnings.

3. Build alpha N (N=1 to start, then N=2 if you need more commits, etc.) and
upload to pypi. See "Building and uploading the release wheels" below for
Expand Down
7 changes: 7 additions & 0 deletions docker-testing/README.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Docker based testing
--------------------

This directory contains helper files used for running west's tests in Docker on
various Linux runtimes. It was originally developed for release testing.

Run "./run-tests.sh" in this directory to run the tests.
9 changes: 9 additions & 0 deletions docker-testing/arch/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM archlinux:latest
CMD ["/west/docker-testing/in-container-test.sh"]

RUN pacman -Syu --noconfirm \
git \
python-pip \
&& pacman -Scc --noconfirm

RUN pip3 install --break-system-packages tox
76 changes: 76 additions & 0 deletions docker-testing/compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# It would be nicer to consolidate the common user/volumes
# boilerplate, but there wasn't enough time to figure out if it was
# possible at time of writing. Improvements welcome.

services:
west-arch:
build: arch
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/arch
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/arch

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west

west-debian-12:
build: debian-12
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/debian-12
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/debian-12

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west

west-debian-testing:
build: debian-testing
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/debian-testing
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/debian-testing

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west

west-fedora-38:
build: fedora-38
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/fedora-38
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/fedora-38

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west

west-ubuntu-22.04:
build: ubuntu-22.04
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/ubuntu-22.04
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/ubuntu-22.04

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west

west-ubuntu-23.04:
build: ubuntu-23.04
environment:
WEST_TOX_OUT: /west/docker-testing/outdir/ubuntu-23.04
WEST_TOX_OUT_IN_HOST: ${WEST_IN_HOST}/docker-testing/outdir/ubuntu-23.04

user: ${MY_UID}:${MY_GID}
volumes:
- /etc/passwd:/etc/passwd:ro
- /etc/group:/etc/group:ro
- ..:/west
10 changes: 10 additions & 0 deletions docker-testing/debian-12/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM debian:bookworm
CMD ["/west/docker-testing/in-container-test.sh"]

RUN apt-get update \
&& apt-get install -y \
git \
python3-pip \
&& rm -rf /var/lib/apt/lists/*

RUN pip3 install --break-system-packages tox
10 changes: 10 additions & 0 deletions docker-testing/debian-testing/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM debian:testing
CMD ["/west/docker-testing/in-container-test.sh"]

RUN apt-get update \
&& apt-get install -y \
git \
python3-pip \
&& rm -rf /var/lib/apt/lists/*

RUN pip3 install --break-system-packages tox
9 changes: 9 additions & 0 deletions docker-testing/fedora-38/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM fedora:38
CMD ["/west/docker-testing/in-container-test.sh"]

RUN dnf install -y \
git \
python3-pip \
&& dnf clean dbcache

RUN pip3 install tox
38 changes: 38 additions & 0 deletions docker-testing/in-container-test.sh
Copy link
Collaborator

@marc-hb marc-hb Sep 28, 2023

Choose a reason for hiding this comment

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

Forgot this: this script is more than big enough to deserve a main() function.

Very small change, all the benefits listed in:

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#!/bin/bash

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
set -e

It does not catch everything but it catches a lot.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its omission is intentional in this file -- see error handling being done manually below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

set -e is not incompatible with manual error handling at all. It would be totally unusable otherwise. set -e catches many failures that the developer did not anticipate or was too lazy or too optimistic to handle manually - now and in the future.

The only problem with set -e is the false, 100% safety impression that it can give to some inexperienced people. For everything else it's fine.

thesofproject/sof-test#312

# This is the test script that runs in the containers themselves.

WEST=/west

die() {
if [ ! -z "$@" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

"$@" expands to multiple strings. That's not what you want here
http://mywiki.wooledge.org/BashPitfalls#for_arg_in_.24.2A
http://mywiki.wooledge.org/BashGuide/Parameters#Special_Parameters_and_Variables

Suggested change
if [ ! -z "$@" ]; then
if [ -n "$*" ]; then

Or even just:

Suggested change
if [ ! -z "$@" ]; then
if [ -n "$1" ]; then

Or if [ $# -eq 0 ]

Then for consistency:

echo "error: $*" >&2

Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW you don't HAVE to printf 'error: %s' "$var", you can still do printf "error: $var" if you want. The well-tested die() function I recommended earlier supports either indifferently.

echo "error: $@" >&2
else
echo "error: unknown error in $0"
fi
exit 1
}

main()
{
# Verify the container environment set up meets this script's requirements.
[ ! -z "$WEST_TOX_OUT" ] || die "missing $WEST_TOX_OUT"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
[ ! -z "$WEST_TOX_OUT" ] || die "missing $WEST_TOX_OUT"
[ -n "$WEST_TOX_OUT" ] || die "missing $WEST_TOX_OUT"

The fewer negations the better and copy/paste of ! can trigger history recall in interactive testing on the command line.

[ ! -z "$WEST_TOX_OUT_IN_HOST" ] || die "missing $WEST_TOX_OUT_IN_HOST"
[ -d "$WEST" ] || die "missing $WEST in the container"

TOX_LOG="$WEST_TOX_OUT/tox.log"
TOX_LOG_IN_HOST="$WEST_TOX_OUT_IN_HOST/tox.log"
WEST_TESTDIR="/tmp/west"

mkdir "$WEST_TOX_OUT" || die "failed to make $WEST_TOX_OUT in container ($WEST_TOX_OUT_IN_HOST in host)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit

Suggested change
mkdir "$WEST_TOX_OUT" || die "failed to make $WEST_TOX_OUT in container ($WEST_TOX_OUT_IN_HOST in host)"
mkdir "$WEST_TOX_OUT" ||
die "failed to make $WEST_TOX_OUT in container ($WEST_TOX_OUT_IN_HOST in host)"

(No need for a backslash in this case)

Copy link
Collaborator

Choose a reason for hiding this comment

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

How likely is this mkdir to fail really? Isn't set -e enough?


git clone -q "$WEST" "$WEST_TESTDIR" || die "failed to clone west to $WEST_TESTDIR in container"
cd "$WEST_TESTDIR"

echo "running tox, output in $TOX_LOG_IN_HOST in host"
tox run >"$TOX_LOG" 2>&1 || die "tox failed"

cp -R htmlcov "$WEST_TOX_OUT" || die "failed to copy coverage to $WEST_TOX_OUT_IN_HOST/htmlcov in host"
}

main "$@"
14 changes: 14 additions & 0 deletions docker-testing/run-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/bin/bash

# This is the top-level test script that runs in the host.

HERE=$(dirname "$0")

[ -d "$HERE/outdir" ] && rm -r "$HERE/outdir"

set -e
mkdir "$HERE/outdir"
export MY_UID=$(id -u)
export MY_GID=$(id -g)
export WEST_IN_HOST=$(realpath "$HERE/..")
docker-compose up --force-recreate --build
10 changes: 10 additions & 0 deletions docker-testing/ubuntu-22.04/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM ubuntu:22.04
CMD ["/west/docker-testing/in-container-test.sh"]

RUN apt-get update \
&& apt-get install -y \
git \
python3-pip \
&& rm -rf /var/lib/apt/lists/*

RUN pip3 install tox
10 changes: 10 additions & 0 deletions docker-testing/ubuntu-23.04/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
FROM ubuntu:23.04
CMD ["/west/docker-testing/in-container-test.sh"]

RUN apt-get update \
&& apt-get install -y \
git \
python3-pip \
&& rm -rf /var/lib/apt/lists/*

RUN pip3 install --break-system-packages tox
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ deps =
setenv =
TOXTEMPDIR={envtmpdir}
commands =
python -m pytest --cov=west {posargs:tests}
python -m pytest --cov-report=html --cov=west {posargs:tests}
python -m flake8 --config={toxinidir}/tox.ini {toxinidir}
python -m mypy --config-file={toxinidir}/tox.ini --package=west
Loading