Rewrite tests in Python using pytest

.gitignore

@@ -4,3 +4,6 @@ dist/
 *.egg-info
 .tox
 build/
+
+__pycache__/
+*.py[cod]

.pre-commit-config.yaml

@@ -2,24 +2,31 @@
     sha: 003e43251aea1da33f2072f2365ec8b9ceaae070
     hooks:
     -   id: autopep8-wrapper
+        language_version: python2.7
     -   id: check-added-large-files
     -   id: check-docstring-first
+        language_version: python2.7
     -   id: check-json
     -   id: check-merge-conflict
     -   id: check-xml
     -   id: check-yaml
     -   id: debug-statements
+        language_version: python2.7
     -   id: detect-private-key
     -   id: double-quote-string-fixer
+        language_version: python2.7
     -   id: end-of-file-fixer
     -   id: flake8
+        language_version: python2.7
     -   id: name-tests-test
+        exclude: ^tests/lib
     -   id: requirements-txt-fixer
     -   id: trailing-whitespace
 -   repo: https://github.com/asottile/reorder_python_imports.git
     sha: 3d86483455ab5bd06cc1069fdd5ac57be5463f10
     hooks:
     -   id: reorder-python-imports
+        language_version: python2.7
 -   repo: https://github.com/Lucas-C/pre-commit-hooks.git
     sha: 181a63c511691da58116fa19a7241956018660bc
     hooks:

Makefile

@@ -1,28 +1,30 @@
 CFLAGS=-std=gnu99 -static -Wall -Werror
 
-TEST_PACKAGE_DEPS := python procps psmisc libpcre3
+TEST_PACKAGE_DEPS := python python-pip
 
 DOCKER_RUN_TEST := docker run -v $(PWD):/mnt:ro
 DOCKER_DEB_TEST := sh -euxc ' \
 	apt-get update \
 	&& apt-get install -y --no-install-recommends $(TEST_PACKAGE_DEPS) \
-	&& (which timeout || apt-get install -y --no-install-recommends timeout) \
 	&& dpkg -i /mnt/dist/*.deb \
-	&& cd /mnt \
+	&& tmp=$$(mktemp -d) \
-	&& ./test \
+	&& cp -r /mnt/* "$$tmp" \
+	&& cd "$$tmp" \
+	&& pip install pytest \
+	&& py.test tests/ \
 	&& exec dumb-init /mnt/tests/test-zombies \
 '
 DOCKER_PYTHON_TEST := sh -uexc ' \
 	apt-get update \
 	&& apt-get install -y --no-install-recommends python-pip build-essential $(TEST_PACKAGE_DEPS) \
-	&& (which timeout || apt-get install -y --no-install-recommends timeout) \
 	&& tmp=$$(mktemp -d) \
 	&& cp -r /mnt/* "$$tmp" \
 	&& cd "$$tmp" \
 	&& python setup.py clean \
 	&& python setup.py sdist \
 	&& pip install -vv dist/*.tar.gz \
-	&& ./test \
+	&& pip install pytest \
+	&& py.test tests/ \
 	&& exec dumb-init /mnt/tests/test-zombies \
 '
 

debian/control

@@ -3,8 +3,7 @@ Section: utils
 Priority: extra
 Maintainer: Chris Kuehl <ckuehl@yelp.com>
 Uploaders: Kent Wills <rkwills@yelp.com>
-Build-Depends: debhelper (>= 7), gcc, fakeroot, procps, psmisc, libpcre3,
+Build-Depends: debhelper (>= 7), gcc, fakeroot, python, python-pytest
- python
 Standards-Version: 3.9.6
 
 Package: dumb-init

debian/rules

@@ -6,4 +6,6 @@ override_dh_builddeb:
 	dh_builddeb -- -Zgzip
 
 override_dh_auto_test:
-	./test ./dumb-init
+	find . -name '*.pyc' -delete
+	PATH=.:$$PATH py.test tests/
+	ps aux

requirements-dev.txt

@@ -0,0 +1,2 @@
+pre-commit>=0.5.0
+pytest

test

@@ -1,34 +0,0 @@
-#!/bin/bash -eux
-if [ "$#" -eq 1 ]; then
-    dumb_init_bin=$(readlink -f "$1")
-else
-    dumb_init_bin=$(which dumb-init) || {
-        echo "Couldn't find dumb-init on your path, exiting."
-        exit 1
-    }
-fi
-
-echo "Running with dumb-init at '$dumb_init_bin'"
-
-run_tests() {
-    ./test-tty "$dumb_init_bin"
-    
-    export DUMB_INIT_SETSID
-    for DUMB_INIT_SETSID in 0 1; do
-        ./test-proxies-signals "$dumb_init_bin"
-        ./test-exit-status "$dumb_init_bin"
-        ./test-help-message "$dumb_init_bin"
-    done
-
-    DUMB_INIT_SETSID=0 ./test-setsid "$dumb_init_bin" 4
-    DUMB_INIT_SETSID=1 ./test-setsid "$dumb_init_bin" 0
-}
-
-cd tests
-
-echo "Running tests in normal mode."
-run_tests
-
-echo "Running tests in debug mode."
-export DUMB_INIT_DEBUG=1
-run_tests

tests/child_processes_test.py

@@ -0,0 +1,51 @@
+import os
+import signal
+import time
+from subprocess import Popen
+
+from tests.lib.testing import is_alive
+from tests.lib.testing import pid_tree
+
+
+def spawn_and_kill_pipeline():
+    proc = Popen((
+        'dumb-init',
+        'sh', '-c',
+        "yes 'oh, hi' | tail & yes error | tail >&2"
+    ))
+    time.sleep(0.1)
+
+    pids = pid_tree(os.getpid())
+    assert len(living_pids(pids)) == 6
+
+    proc.send_signal(signal.SIGTERM)
+    proc.wait()
+
+    time.sleep(0.1)
+    return pids
+
+
+def living_pids(pids):
+    return {pid for pid in pids if is_alive(pid)}
+
+
+def test_setsid_signals_entire_group(both_debug_modes):
+    """When dumb-init is running in setsid mode, it should only signal the
+    entire process group rooted at it.
+    """
+    os.environ['DUMB_INIT_SETSID'] = '1'
+    pids = spawn_and_kill_pipeline()
+    assert len(living_pids(pids)) == 0
+
+
+def test_no_setsid_doesnt_signal_entire_group(both_debug_modes):
+    """When dumb-init is not running in setsid mode, it should only signal its
+    immediate child.
+    """
+    os.environ['DUMB_INIT_SETSID'] = '0'
+    pids = spawn_and_kill_pipeline()
+
+    living = living_pids(pids)
+    assert len(living) == 4
+    for pid in living:
+        os.kill(pid, signal.SIGKILL)

tests/conftest.py

@@ -0,0 +1,13 @@
+import os
+
+import pytest
+
+
+@pytest.fixture(params=['1', '0'])
+def both_debug_modes(request):
+    os.environ['DUMB_INIT_DEBUG'] = request.param
+
+
+@pytest.fixture(params=['1', '0'])
+def both_setsid_modes(request):
+    os.environ['DUMB_INIT_SETSID'] = request.param

tests/exit_status_test.py

@@ -0,0 +1,11 @@
+from subprocess import Popen
+
+
+def test_exit_status(both_debug_modes, both_setsid_modes):
+    """dumb-init should exit with the same exit status as the process that it
+    supervises.
+    """
+    for status in [0, 1, 2, 32, 64, 127, 254, 255]:
+        proc = Popen(('dumb-init', 'sh', '-c', 'exit {}'.format(status)))
+        proc.wait()
+        assert proc.returncode == status

tests/help_message_test.py

@@ -0,0 +1,12 @@
+from subprocess import PIPE
+from subprocess import Popen
+
+
+def test_exit_status(both_debug_modes, both_setsid_modes):
+    """dumb-init should say something useful when called with no arguments, and
+    exit nonzero.
+    """
+    proc = Popen(('dumb-init'), stderr=PIPE)
+    _, stderr = proc.communicate()
+    assert proc.returncode != 0
+    assert len(stderr) >= 50

tests/lib/print-signals

@@ -1,19 +0,0 @@
-#!/bin/sh -eux
-# XXX: We use /bin/sh instead of /bin/bash since some old versions of bash
-# exhibit an issue where they seem to receive the same signal twice.
-# With /bin/sh, this does not seem to happen.
-
-# Print received signals into a file, one per line
-file="$1"
-
-. ./lib/testlib.sh
-
-for i in $(catchable_signals); do
-    trap "echo $i > \"$file\"" "$i"
-done
-
-echo 'ready' > "$file"
-
-echo 'loop forever...'
-set +x
-while :; do true; done

tests/lib/print_signals.py

@@ -0,0 +1,39 @@
+#!/usr/bin/env python
+"""Print received signals to stdout.
+
+Since all signals are printed and otherwise ignored, you'll need to send
+SIGKILL (kill -9) to this process to actually end it.
+"""
+from __future__ import print_function
+
+import signal
+import sys
+import time
+
+from tests.lib.testing import CATCHABLE_SIGNALS
+
+
+print_queue = []
+
+
+def unbuffered_print(line):
+    sys.stdout.write('{}\n'.format(line))
+    sys.stdout.flush()
+
+
+def print_signal(signum, _):
+    print_queue.append(signum)
+
+
+if __name__ == '__main__':
+    for signum in CATCHABLE_SIGNALS:
+        signal.signal(signum, print_signal)
+
+    unbuffered_print('ready')
+
+    # loop forever just printing signals
+    while True:
+        if print_queue:
+            unbuffered_print(print_queue.pop())
+
+        time.sleep(0.01)

tests/lib/testing.py

@@ -0,0 +1,34 @@
+import signal
+
+from py._path.local import LocalPath
+
+
+CATCHABLE_SIGNALS = frozenset(
+    set(range(1, 32)) - {signal.SIGKILL, signal.SIGSTOP, signal.SIGCHLD}
+)
+
+
+def child_pids(pid):
+    """Return a list of direct child PIDs for the given PID."""
+    pid = str(pid)
+    tasks = LocalPath('/proc').join(pid, 'task').listdir()
+    return {
+        int(child_pid)
+        for task in tasks
+        for child_pid in task.join('children').read().split()
+    }
+
+
+def pid_tree(pid):
+    """Return a list of all descendant PIDs for the given PID."""
+    children = child_pids(pid)
+    return {
+        pid
+        for child in children
+        for pid in pid_tree(child)
+    } | children
+
+
+def is_alive(pid):
+    """Return whether a process is running with the given PID."""
+    return LocalPath('/proc').join(str(pid)).isdir()

tests/lib/testlib.sh

@@ -1,4 +0,0 @@
-catchable_signals() {
-    # We can't handle the signals SIGKILL=9, SIGCHLD=17, SIGSTOP=19
-    seq 1 31 | grep -vE '^(9|17|19)$'
-}

tests/proxies_signals_test.py

@@ -0,0 +1,24 @@
+import os
+import signal
+import sys
+from subprocess import PIPE
+from subprocess import Popen
+
+from tests.lib.testing import CATCHABLE_SIGNALS
+from tests.lib.testing import pid_tree
+
+
+def test_prints_signals(both_debug_modes, both_setsid_modes):
+    proc = Popen(
+        ('dumb-init', sys.executable, '-m', 'tests.lib.print_signals'),
+        stdout=PIPE,
+    )
+
+    assert proc.stdout.readline() == b'ready\n'
+
+    for signum in CATCHABLE_SIGNALS:
+        proc.send_signal(signum)
+        assert proc.stdout.readline() == '{}\n'.format(signum).encode('ascii')
+
+    for pid in pid_tree(proc.pid):
+        os.kill(pid, signal.SIGKILL)

tests/test-exit-status

@@ -1,12 +0,0 @@
-#!/bin/bash -eux
-dumb_init="$1"
-
-# dumb-init should exit with the same exit status as the process it launches.
-for i in 0 1 2 32 64 127 254 255; do
-    status=$($dumb_init sh -c "exit $i"; echo $?)
-
-    if [ "$status" -ne "$i" ]; then
-        echo "Error: Expected exit status $i, got $status."
-        exit 1
-    fi
-done

tests/test-help-message

@@ -1,20 +0,0 @@
-#!/bin/bash -eux
-# dumb-init should say something useful when called with no arguments, and exit
-# nonzero.
-
-dumb_init="$1"
-
-status=$($dumb_init > /dev/null 2>&1; echo $?)
-
-if [ "$status" -eq 0 ]; then
-    echo "Error: Expected dumb-init with no arguments to return nonzero, but it returned ${status}."
-    exit 1
-fi
-
-msg=$($dumb_init 2>&1 || true)
-msg_len=${#msg}
-
-if [ "$msg_len" -le 50 ]; then
-    echo "Error: Expected dumb-init with no arguments to print a useful message, but it was only ${msg_len} chars long."
-    exit 1
-fi

tests/test-proxies-signals

@@ -1,46 +0,0 @@
-#!/bin/bash -euxm
-# dumb-init should proxy all possible signals to the child process.
-dumb_init="$1"
-
-# Try sending all signals via dumb-init to our `print-signals` script, ensure
-# they were all received.
-. ./lib/testlib.sh
-
-# The easiest way to communicate with the background process is with a FIFO.
-# (piping spawns additional subshells and makes it hard to get the right PID)
-fifo=$(mktemp -u)
-mkfifo -m 600 "$fifo"
-read_cmd="timeout 1 head -n1 $fifo"
-
-$dumb_init ./lib/print-signals "$fifo" &
-pid="$!"
-
-# Wait for `print-signals` to indicate it's ready.
-$read_cmd > /dev/null
-
-for expected in $(catchable_signals); do
-    kill -s "$expected" "$pid"
-    echo -n "Sent signal ${expected}... "
-    received=$($read_cmd) || {
-        echo
-        echo "Error: Didn't receive signal within 1 second."
-        exit 1
-    }
-
-    echo "received signal ${received}."
-
-    if [ "$expected" -ne "$received" ]; then
-        echo "Error: Received signal $received, but expected signal $expected."
-        exit 1
-    fi
-done
-
-# Turn off job monitoring so we don't get a spurious "[1] + Killed" printed
-set +m
-
-# $pid is the PID of the dumb-init process. Since print-signals ignores all
-# signals, we need to `kill -9` it. If we just `kill -9` the dumb-init process,
-# `print-signals` will still be running. So we instead kill children of the
-# dumb-init process.
-pkill -9 -P "$pid"
-rm "$fifo"

tests/test-setsid

@@ -1,33 +0,0 @@
-#!/bin/bash -eux
-# dumb-init should proxy signals to a session rooted at its child when
-# requested.
-dumb_init="$1"
-after_count="$2"
-
-$dumb_init sh -c "yes 'oh, hi' | tail & yes error | tail >&2" &
-pid="$!"
-
-sleep 1
-pstree -p "$pid"
-pids=$(pstree -p "$pid" | grep -Po '(\d+)' | grep -Po '\d+')
-
-# ensure processes are running
-child_count=$(ps -o pid= $pids | wc -l) || true
-
-if [ "$child_count" -ne 6 ]; then
-    echo "Error: Expected 6 children, instead we had ${child_count}."
-    exit 1
-fi
-
-# ensure processes are dead after signal
-kill -TERM "$pid"
-sleep 1
-child_count=$(ps -o pid= $pids | wc -l) || true
-
-if [ "$child_count" -ne "$after_count" ]; then
-    echo "Error: Expected $after_count children, instead we had ${child_count}."
-    exit 1
-fi
-
-echo 'Killing any leftover processes.'
-xargs kill -9 <<< "$pids" || true

tests/test-zombies

@@ -4,7 +4,8 @@
 # dumb-init as PID1.
 #
 # We run it as the last step of the integration tests inside our Docker
-# containers.
+# containers. Since this test must run as PID 1, we don't use pytest and
+# instead write it in bash.
 
 bash -euxc "bash -euxc 'echo i am a zombie' &" &
 

tests/tty_test.py

@@ -1,10 +1,13 @@
-#!/usr/bin/env python
+import os
+
+
 EOF = b'\x04'
 
 
 def ttyflags(fd):
     """normalize tty i/o for testing"""
-    # see: http://www.gnu.org/software/libc/manual/html_mono/libc.html#Output-Modes
+    # see:
+    # http://www.gnu.org/software/libc/manual/html_mono/libc.html#Output-Modes
     import termios as T
     attrs = T.tcgetattr(fd)
     attrs[1] &= ~T.OPOST  # don't munge output
@@ -12,13 +15,13 @@ def ttyflags(fd):
     T.tcsetattr(fd, T.TCSANOW, attrs)
 
 
-def tac(dumb_init):
+def tac():
     """
     run tac. if it fails to complete in 1 second send SIGKILL and exit with an
     error.
     """
     from os import execvp
-    execvp('timeout', ('timeout', '1', dumb_init, 'tac'))
+    execvp('timeout', ('timeout', '1', 'dumb-init', 'tac'))
 
 
 def readall(fd):
@@ -39,7 +42,7 @@ def readall(fd):
             result += chunk
 
 
-def test(fd):
+def _test(fd):
     """write to tac via the pty and verify its output"""
     ttyflags(fd)
     from os import write
@@ -50,23 +53,15 @@ def test(fd):
     print('PASS')
 
 
-def test_tty(dumb_init):
+def test_tty():
     """
     Ensure processes wrapped by dumb-init can write successfully, given a tty
     """
+    # disable debug output so it doesn't break our assertion
+    os.environ['DUMB_INIT_DEBUG'] = '0'
     import pty
     pid, fd = pty.fork()
     if pid == 0:
-        tac(dumb_init)
+        tac()
     else:
-        test(fd)
+        _test(fd)
-
-
-def main():
-    from os import environ
-    environ['DUMB_INIT_DEBUG'] = '0'  # disable debug output so it doesn't break our assertion
-    from sys import argv
-    test_tty(argv[1])
-
-
-main()

tox.ini

@@ -1,10 +1,10 @@
 [tox]
-envlist = py27,py33,py34
+envlist = py27,py34
 
 [testenv]
-deps = pre-commit>=0.5.0
+deps = -rrequirements-dev.txt
 commands =
-    ./test
+    python -m pytest
     pre-commit run --all-files
 
 [testenv:pre-commit]