Major overhaul of the test suite

Signed-off-by: yubiuser <github@yubiuser.dev>

.github/workflows/build-and-test.yml

@@ -3,30 +3,49 @@ on:
   pull_request:
 
 jobs:
-  test:
-    runs-on: ubuntu-latest
+  build-and-test:
+    runs-on: ${{ matrix.runner }}
     strategy:
       fail-fast: false
       matrix:
-        # Official docker images for docker are only available for amd64 and arm64
-        # TODO: Look at: https://github.com/docker-library/official-images#architectures-other-than-amd64
-        # Is testing on all platforms really necessary?
-        # Disabled arm64 tests for the time being, something is wrong with the test config and the volumes are getting shared between the test containers on different architectures
-        #platform: [linux/amd64, linux/arm64]
-        platform: [linux/amd64]
+        include:
+          - platform: linux/amd64
+            runner: ubuntu-latest
+          - platform: linux/386
+            runner: ubuntu-latest
+          - platform: linux/arm/v6
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm/v7
+            runner: ubuntu-24.04-arm
+          - platform: linux/arm64
+            runner: ubuntu-24.04-arm
+          - platform: linux/riscv64
+            runner: ubuntu-24.04-arm
+    env:
+      CI_ARCH: ${{ matrix.platform }}
     steps:
     - name: Checkout Repo
       uses: actions/checkout@v4
 
     - name: Set up QEMU
       uses: docker/setup-qemu-action@v3
+
+    - name: Set up Python
+      uses: actions/setup-python@v5.6.0
       with:
-        platforms: ${{ matrix.platform }}
-
-    - name: Set up Docker Buildx
-      uses: docker/setup-buildx-action@v3
-
-    - name: Run Tests
+        python-version: "3.13"
+    
+    - name: Run black formatter
       run: |
-        echo "Building image to test"
-        PLATFORM=${{ matrix.platform }} ./build-and-test.sh
+        pip install black
+        black --check --diff test/tests/
+
+    - name: Install wheel
+      run:  pip install wheel
+
+    - name: Install dependencies
+      run: pip install -r test/requirements.txt
+
+    - name: Test with tox
+      run: |
+        CIPLATFORM=${{ env.CI_ARCH }} tox -c test/tox.ini

build-and-test.sh

@@ -1,24 +0,0 @@
-#!/usr/bin/env bash
-set -ex
-
-if [[ "$1" == "enter" ]]; then
-    enter="-it"
-    cmd="sh"
-fi
-
-GIT_BRANCH=$(git rev-parse --abbrev-ref HEAD | sed "s/\//-/g")
-GIT_TAG=$(git describe --tags --exact-match 2>/dev/null || true)
-GIT_TAG="${GIT_TAG:-$GIT_BRANCH}"
-PLATFORM="${PLATFORM:-linux/amd64}"
-
-# generate and build dockerfile
-docker buildx build --load --platform=${PLATFORM} --tag image_pipenv --file test/Dockerfile test/
-docker run --rm \
-    --volume /var/run/docker.sock:/var/run/docker.sock \
-    --volume "$(pwd):/$(pwd)" \
-    --workdir "$(pwd)" \
-    --env PIPENV_CACHE_DIR="$(pwd)/.pipenv" \
-    --env GIT_TAG="${GIT_TAG}" \
-    --env PY_COLORS=1 \
-    --env TARGETPLATFORM="${PLATFORM}" \
-    ${enter} image_pipenv ${cmd}

test/Dockerfile

@@ -1,22 +0,0 @@
-ARG alpine_version="3.22"
-ARG docker_version="28.2.2"
-
-FROM docker:${docker_version}-cli-alpine${alpine_version}
-
-COPY --chmod=0755 ./cmd.sh /usr/local/bin/cmd.sh
-COPY requirements.txt /root/
-WORKDIR /root
-
-RUN apk add --no-cache \
-    python3-dev \
-    py3-pip \
-    curl \
-    && pip3 install --break-system-packages --no-cache-dir -U pip  \
-    && pip3 install --break-system-packages --no-cache-dir -r requirements.txt \
-    # Replace hardcoded /bin/sh with /bin/bash in testinfra docker backend
-    # see https://github.com/pytest-dev/pytest-testinfra/issues/582 and similar issues
-    && pythonversion=$(python3 -c 'import sys; print(".".join(map(str, sys.version_info[:2])))') \
-    && sed -i 's|/bin/sh|/bin/bash|g' /usr/lib/python${pythonversion}/site-packages/testinfra/backend/docker.py
-
-SHELL ["/bin/sh", "-c"]
-CMD ["cmd.sh"]

test/TESTING.md

@@ -1,19 +1,12 @@
 # Prerequisites
 
-Make sure you have bash & docker installed.
-Python and some test hacks are crammed into the `Dockerfile_build` file for now.
-Revisions in the future may re-enable running python on your host (not just in docker).
+Make sure you have `docker`, `python` and `tox` installed.
 
 # Running tests locally
 
-`./build-and-test.sh`
+`tox -c test/tox.ini`
 
 Should result in:
 
-- An image named `pihole:[branch-name]` being built
+- An image named `pihole:CI_container` being built
 - Tests being ran to confirm the image doesn't have any regressions
-
-# Modify Pipfile
-
-You can enter into the test docker image using `./build-and-test.sh enter`.
-From there, you can `cd test` and execute any needed pipenv commands.

test/cmd.sh

@@ -1,11 +0,0 @@
-#!/usr/bin/env sh
-set -eux
-
-docker buildx build ./src --build-arg TARGETPLATFORM="${TARGETPLATFORM}" --tag pihole:${GIT_TAG} --no-cache
-docker images pihole:${GIT_TAG}
-
-# auto-format the pytest code
-python3 -m black ./test/tests/
-
-# TODO: Add junitxml output and have something consume it
-COLUMNS=120 py.test -vv -n auto ./test/tests/

test/requirements.txt

@@ -1,5 +1,6 @@
 pytest == 8.4.1
-pytest-xdist == 3.7.0
 pytest-testinfra == 10.2.2
-black == 25.1.0
 pytest-clarity == 1.0.1
+tox == 4.27.0
+# Not adding pytest-xdist as using pytest with n > 1 cores 
+# causes random issues with the emulated architectures

test/tests/conftest.py

@@ -1,131 +1,64 @@
-import os
 import pytest
 import subprocess
 import testinfra
-
-local_host = testinfra.get_host("local://")
-check_output = local_host.check_output
-
-TAIL_DEV_NULL = "tail -f /dev/null"
+import testinfra.backend.docker
+import os
 
 
-@pytest.fixture()
-def run_and_stream_command_output():
-    def run_and_stream_command_output_inner(command, verbose=False):
-        print("Running", command)
-        build_env = os.environ.copy()
-        build_env["PIHOLE_DOCKER_TAG"] = version
-        build_result = subprocess.Popen(
-            command.split(),
-            env=build_env,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.STDOUT,
-            bufsize=1,
-            universal_newlines=True,
+# Monkeypatch sh to bash, if they ever support non hard code /bin/sh this can go away
+# https://github.com/pytest-dev/pytest-testinfra/blob/master/testinfra/backend/docker.py
+def run_bash(self, command, *args, **kwargs):
+    cmd = self.get_command(command, *args)
+    if self.user is not None:
+        out = self.run_local(
+            "docker exec -u %s %s /bin/bash -c %s", self.user, self.name, cmd
         )
-        if verbose:
-            while build_result.poll() is None:
-                for line in build_result.stdout:
-                    print(line, end="")
-        build_result.wait()
-        if build_result.returncode != 0:
-            print(f"       [i] Error running: {command}")
-            print(build_result.stderr)
-
-    return run_and_stream_command_output_inner
+    else:
+        out = self.run_local("docker exec %s /bin/bash -c %s", self.name, cmd)
+    out.command = self.encode(cmd)
+    return out
 
 
-@pytest.fixture()
-def args_env():
-    return '-e TZ="Europe/London" -e FTLCONF_dns_upstreams="8.8.8.8"'
+testinfra.backend.docker.DockerBackend.run = run_bash
 
 
-@pytest.fixture()
-def args(args_env):
-    return "{}".format(args_env)
+# scope='session' uses the same container for all the tests;
+# scope='function' uses a new container per test function.
+@pytest.fixture(scope="function")
+def docker(request):
+    # Get platform from environment variable, default to None if not set
+    platform = os.environ.get("CIPLATFORM")
 
+    # build the docker run command with args
+    cmd = ["docker", "run", "-d", "-t"]
 
-@pytest.fixture()
-def test_args():
-    """test override fixture to provide arguments separate from our core args"""
-    return ""
+    # Only add platform flag if CIPLATFORM is set
+    if platform:
+        cmd.extend(["--platform", platform])
 
+    # Get env vars from parameterization
+    env_vars = getattr(request, "param", [])
+    if isinstance(env_vars, str):
+        env_vars = [env_vars]
 
-def docker_generic(request, _test_args, _args, _image, _cmd, _entrypoint):
-    # assert 'docker' in check_output('id'), "Are you in the docker group?"
-    # Always appended PYTEST arg to tell pihole we're testing
-    if "pihole" in _image and "PYTEST=1" not in _args:
-        _args = "{} -e PYTEST=1".format(_args)
-    docker_run = "docker run -d -t {args} {test_args} {entry} {image} {cmd}".format(
-        args=_args, test_args=_test_args, entry=_entrypoint, image=_image, cmd=_cmd
-    )
-    # Print a human runable version of the container run command for faster debugging
-    print(docker_run.replace("-d -t", "--rm -it").replace(TAIL_DEV_NULL, "bash"))
-    docker_id = check_output(docker_run)
+    # add parameterized environment variables
+    for env_var in env_vars:
+        cmd.extend(["-e", env_var])
 
-    def teardown():
-        check_output("docker logs {}".format(docker_id))
-        check_output("docker rm -f {}".format(docker_id))
+    # ensure PYTEST=1 is set
+    if not any("PYTEST=1" in arg for arg in cmd):
+        cmd.extend(["-e", "PYTEST=1"])
 
-    request.addfinalizer(teardown)
-    docker_container = testinfra.backend.get_backend(
-        "docker://" + docker_id, sudo=False
-    )
-    docker_container.id = docker_id
+    # add default TZ if not already set
+    if not any("TZ=" in arg for arg in cmd):
+        cmd.extend(["-e", 'TZ="Europe/London"'])
 
-    return docker_container
+    # add the image name
+    cmd.append("pihole:CI_container")
 
-
-@pytest.fixture
-def docker(request, test_args, args, image, cmd, entrypoint):
-    """One-off Docker container run"""
-    return docker_generic(request, test_args, args, image, cmd, entrypoint)
-
-
-@pytest.fixture
-def entrypoint():
-    return ""
-
-
-@pytest.fixture()
-def version():
-    return os.environ.get("GIT_TAG", None)
-
-
-@pytest.fixture()
-def tag(version):
-    return "{}".format(version)
-
-
-@pytest.fixture()
-def image(tag):
-    image = "pihole"
-    return "{}:{}".format(image, tag)
-
-
-@pytest.fixture()
-def cmd():
-    return TAIL_DEV_NULL
-
-
-@pytest.fixture
-def slow():
-    """
-    Run a slow check, check if the state is correct for `timeout` seconds.
-    """
-    import time
-
-    def _slow(check, timeout=20):
-        timeout_at = time.time() + timeout
-        while True:
-            try:
-                assert check()
-            except AssertionError as e:
-                if time.time() < timeout_at:
-                    time.sleep(1)
-                else:
-                    raise e
-            else:
-                return
-
-    return _slow
+    # run a container
+    docker_id = subprocess.check_output(cmd).decode().strip()
+    # return a testinfra connection to the container
+    yield testinfra.get_host("docker://" + docker_id)
+    # at the end of the test suite, destroy the container
+    subprocess.check_call(["docker", "rm", "-f", docker_id])

test/tests/test_bash_functions.py

@@ -1,17 +1,33 @@
 import pytest
 
 
-@pytest.mark.parametrize("test_args", ['-e "FTLCONF_webserver_port=999"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["FTLCONF_webserver_port=999"], indirect=True)
 def test_ftlconf_webserver_port(docker):
-    func = docker.run("pihole-FTL --config webserver.port")
+    func = docker.run("echo ${FTLCONF_webserver_port}")
+    assert "999" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        pihole-FTL --config webserver.port
+        """
+    )
     assert "999" in func.stdout
 
 
+# Adding 5 seconds sleep to give the emulated architecture time to run
 @pytest.mark.parametrize(
-    "test_args", ['-e "FTLCONF_dns_upstreams=1.2.3.4;5.6.7.8#1234"']
+    "docker", ["FTLCONF_dns_upstreams=1.2.3.4;5.6.7.8#1234"], indirect=True
 )
 def test_ftlconf_dns_upstreams(docker):
-    func = docker.run("pihole-FTL --config dns.upstreams")
+    func = docker.run("echo ${FTLCONF_dns_upstreams}")
+    assert "1.2.3.4;5.6.7.8#1234" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        pihole-FTL --config dns.upstreams
+        """
+    )
     assert "[ 1.2.3.4, 5.6.7.8#1234 ]" in func.stdout
 
 
@@ -24,7 +40,7 @@ def test_random_password_assigned_fresh_start(docker):
 
 
 @pytest.mark.parametrize(
-    "test_args", ['-e "FTLCONF_webserver_api_password=1234567890"']
+    "docker", ["FTLCONF_webserver_api_password=1234567890"], indirect=True
 )
 def test_password_set_by_envvar(docker):
     func = docker.run(CMD_SETUP_WEB_PASSWORD)

test/tests/test_general.py

@@ -1,15 +1,32 @@
 import pytest
+import os
 
 
-@pytest.mark.parametrize("test_args", ['-e "PIHOLE_UID=456"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["PIHOLE_UID=456"], indirect=True)
 def test_pihole_uid_env_var(docker):
-    func = docker.run("id -u pihole")
+    func = docker.run("echo ${PIHOLE_UID}")
+    assert "456" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        id -u pihole
+        """
+    )
     assert "456" in func.stdout
 
 
-@pytest.mark.parametrize("test_args", ['-e "PIHOLE_GID=456"'])
+# Adding 5 seconds sleep to give the emulated architecture time to run
+@pytest.mark.parametrize("docker", ["PIHOLE_GID=456"], indirect=True)
 def test_pihole_gid_env_var(docker):
-    func = docker.run("id -g pihole")
+    func = docker.run("echo ${PIHOLE_GID}")
+    assert "456" in func.stdout
+    func = docker.run(
+        """
+        sleep 5
+        id -g pihole
+        """
+    )
     assert "456" in func.stdout
 
 
@@ -19,6 +36,19 @@ def test_pihole_ftl_version(docker):
     assert "Version:" in func.stdout
 
 
+@pytest.mark.skipif(
+    not os.environ.get("CIPLATFORM"),
+    reason="CIPLATFORM environment variable not set, running locally",
+)
+def test_pihole_ftl_architecture(docker):
+    func = docker.run("pihole-FTL -vv")
+    assert func.rc == 0
+    assert "Architecture:" in func.stdout
+    # Get the expected architecture from CIPLATFORM environment variable
+    platform = os.environ.get("CIPLATFORM")
+    assert platform in func.stdout
+
+
 # Wait 5 seconds for startup, then kill the start.sh script
 # Finally, grep the FTL log to see if it has been shut down cleanly
 def test_pihole_ftl_clean_shutdown(docker):

test/tox.ini

@@ -0,0 +1,16 @@
+[tox]
+envlist = py3
+
+[testenv:py3]
+allowlist_externals = docker
+deps = -rrequirements.txt
+passenv = CIPLATFORM
+setenv =
+    COLUMNS=120
+    PY_COLORS=1 
+commands =  # Build the Docker image for testing depending on the architecture, fall back to 'local' if not set
+            # This allows us to run the tests on the host architecture if not on CI
+            docker buildx build --load --platform={env:CIPLATFORM:local} --progress plain -f ../src/Dockerfile -t pihole:CI_container ../src/
+            # run the tests
+            # # Not using > 1 cores as it causes random issues with the emulated architectures
+            pytest {posargs:-vv} ./tests/