From dded88acb9320e3f16a97073c84a139a56e0def5 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 21 Oct 2021 15:59:57 +0200 Subject: [PATCH 1/5] test: wait a bit for the given PID to die if it's still alive When playing around with the coverage-enabled build I kept hitting an issue where dnsmasq failed to start because the previous instance was still shutting down. This should, hopefully, help to mitigate that. --- test/test-network/systemd-networkd-tests.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 733a8d5aa5..4a358edcd2 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -3,6 +3,7 @@ # systemd-networkd tests import argparse +import errno import itertools import os import re @@ -483,6 +484,15 @@ def stop_by_pid_file(pid_file): with open(pid_file, 'r') as f: pid = f.read().rstrip(' \t\r\n\0') os.kill(int(pid), signal.SIGTERM) + for _ in range(25): + try: + os.kill(int(pid), 0) + print(f"PID {pid} is still alive, waiting...") + time.sleep(.2) + except OSError as e: + if e.errno == errno.ESRCH: + break + print(f"Unexpected exception when waiting for {pid} to die: {e.errno}") os.remove(pid_file) From 6c9efba67715cd1ced170ac46c04d47934ad276a Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 21 Oct 2021 17:34:43 +0200 Subject: [PATCH 2/5] test: loosen certain sandbox restrictions when collecting coverage With `ProtectSystem=strict` gcov is unable to write the *.gcda files with collected coverage. Let's add a yet another switch to make such restriction less strict to make gcov happy. This addresses following errors: ``` ... systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/binfmt-util.c.gcda:Cannot open systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/base-filesystem.c.gcda:Cannot open systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/barrier.c.gcda:Cannot open systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/ask-password-api.c.gcda:Cannot open systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/apparmor-util.c.gcda:Cannot open systemd-networkd[272469]: profiling:/systemd-meson-build/src/shared/libsystemd-shared-249.a.p/acpi-fpdt.c.gcda:Cannot open ... ``` --- test/test-network/systemd-networkd-tests.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/test-network/systemd-networkd-tests.py b/test/test-network/systemd-networkd-tests.py index 4a358edcd2..aa81017072 100755 --- a/test/test-network/systemd-networkd-tests.py +++ b/test/test-network/systemd-networkd-tests.py @@ -43,6 +43,7 @@ env = {} asan_options=None lsan_options=None ubsan_options=None +with_coverage=False running_units = [] @@ -305,6 +306,8 @@ def setUpModule(): drop_in += ['SystemCallFilter='] if use_valgrind or asan_options or lsan_options or ubsan_options: drop_in += ['MemoryDenyWriteExecute=no'] + if with_coverage: + drop_in += ['ProtectSystem=no'] os.makedirs('/run/systemd/system/systemd-networkd.service.d', exist_ok=True) with open('/run/systemd/system/systemd-networkd.service.d/00-override.conf', mode='w') as f: @@ -331,6 +334,8 @@ def setUpModule(): drop_in += ['SystemCallFilter='] if use_valgrind or asan_options or lsan_options or ubsan_options: drop_in += ['MemoryDenyWriteExecute=no'] + if with_coverage: + drop_in += ['ProtectSystem=no'] os.makedirs('/run/systemd/system/systemd-resolved.service.d', exist_ok=True) with open('/run/systemd/system/systemd-resolved.service.d/00-override.conf', mode='w') as f: @@ -5037,6 +5042,7 @@ if __name__ == '__main__': parser.add_argument('--asan-options', help='ASAN options', dest='asan_options') parser.add_argument('--lsan-options', help='LSAN options', dest='lsan_options') parser.add_argument('--ubsan-options', help='UBSAN options', dest='ubsan_options') + parser.add_argument('--with-coverage', help='Loosen certain sandbox restrictions to make gcov happy', dest='with_coverage', type=bool, nargs='?', const=True, default=with_coverage) ns, args = parser.parse_known_args(namespace=unittest) if ns.build_dir: @@ -5070,6 +5076,7 @@ if __name__ == '__main__': asan_options = ns.asan_options lsan_options = ns.lsan_options ubsan_options = ns.ubsan_options + with_coverage = ns.with_coverage if use_valgrind: networkctl_cmd = ['valgrind', '--track-origins=yes', '--leak-check=full', '--show-leak-kinds=all', networkctl_bin] From 02d7e73013a92c5580023e4e548d19adbc254dbe Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 21 Oct 2021 18:06:36 +0200 Subject: [PATCH 3/5] test: loosen sandbox restrictions for integration tests as well Otherwise we miss quite a lot of coverage (mainly from logind, hostnamed, networkd, and possibly others), since they can't write their reports with `ProtectSystem=strict`. --- test/test-functions | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/test-functions b/test/test-functions index 2557d15c04..54be19ffc0 100644 --- a/test/test-functions +++ b/test/test-functions @@ -239,6 +239,7 @@ is_built_with_asan() { } IS_BUILT_WITH_ASAN=$(is_built_with_asan && echo yes || echo no) +IS_BUILT_WITH_COVERAGE=$(meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'true' && echo yes || echo no) if get_bool "$IS_BUILT_WITH_ASAN"; then STRIP_BINARIES=no @@ -1015,7 +1016,7 @@ install_compiled_systemd() { # If we are doing coverage runs, copy over the binary notes files, as lcov expects to # find them in the same directory as the runtime data counts - if meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'true'; then + if get_bool "$IS_BUILT_WITH_COVERAGE"; then mkdir -p "${initdir}/${BUILD_DIR:?}/" rsync -am --include='*/' --include='*.gcno' --exclude='*' "${BUILD_DIR:?}/" "${initdir}/${BUILD_DIR:?}/" fi @@ -1098,7 +1099,7 @@ install_systemd() { fi # remove unneeded documentation - rm -fr "$initdir"/usr/share/{man,doc} + rm -fr "${initdir:?}"/usr/share/{man,doc} # enable debug logging in PID1 echo LogLevel=debug >>"$initdir/etc/systemd/system.conf" @@ -1107,6 +1108,13 @@ install_systemd() { # Propagate SYSTEMD_UNIT_PATH to user systemd managers mkdir "$initdir/etc/systemd/system/user@.service.d/" echo -e "[Service]\nPassEnvironment=SYSTEMD_UNIT_PATH\n" >"$initdir/etc/systemd/system/user@.service.d/override.conf" + + # When built with gcov, disable ProtectSystem= in the test images, since + # it prevents gcov to write the coverage reports (*.gcda files) + if get_bool "$IS_BUILT_WITH_COVERAGE"; then + mkdir -p "$initdir/etc/systemd/system/service.d/" + echo -e "[Service]\nProtectSystem=no\n" >"$initdir/etc/systemd/system/service.d/override.conf" + fi } get_ldpath() { @@ -1177,7 +1185,7 @@ create_empty_image() { if meson configure "${BUILD_DIR:?}" | grep 'link-.*-shared' | awk '{ print $2 }' | grep -q 'false'; then size=$((size+=200)) fi - if meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'true'; then + if get_bool "$IS_BUILT_WITH_COVERAGE"; then size=$((size+=250)) fi fi @@ -1288,7 +1296,7 @@ check_coverage_reports() { if get_bool "$NO_BUILD"; then return 0 fi - if meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'false'; then + if ! get_bool "$IS_BUILT_WITH_COVERAGE"; then return 0 fi From 9bc10d3109380e76db04ddf961cd5ddb7ed45122 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 21 Oct 2021 18:12:06 +0200 Subject: [PATCH 4/5] test: collect the coverage _before_ unmounting the rootfs d'oh! --- test/TEST-24-CRYPTSETUP/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TEST-24-CRYPTSETUP/test.sh b/test/TEST-24-CRYPTSETUP/test.sh index 68a54924a4..96d255dd96 100755 --- a/test/TEST-24-CRYPTSETUP/test.sh +++ b/test/TEST-24-CRYPTSETUP/test.sh @@ -20,6 +20,7 @@ check_result_qemu() { cryptsetup luksOpen "${LOOPDEV:?}p2" varcrypt <"$TESTDIR/keyfile" mount /dev/mapper/varcrypt "$initdir/var" save_journal "$initdir/var/log/journal" + check_coverage_reports "${initdir:?}" || ret=5 _umount_dir "$initdir/var" _umount_dir "$initdir" cryptsetup luksClose /dev/mapper/varcrypt @@ -28,7 +29,6 @@ check_result_qemu() { echo "${JOURNAL_LIST:-No journals were saved}" test -s "$TESTDIR/failed" && ret=1 - check_coverage_reports "${initdir:?}" || ret=5 return $ret } From 7a6c5b6ab502167a966a5418073fcf25b134a8ba Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 21 Oct 2021 18:34:21 +0200 Subject: [PATCH 5/5] test: make the coverage check safer for non-compiled builds --- test/test-functions | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/test-functions b/test/test-functions index 54be19ffc0..98b60d17f8 100644 --- a/test/test-functions +++ b/test/test-functions @@ -238,8 +238,16 @@ is_built_with_asan() { fi } +is_built_with_coverage() { + if get_bool "${NO_BUILD:=}" || ! command -v meson >/dev/null; then + return 1 + fi + + meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'true' +} + IS_BUILT_WITH_ASAN=$(is_built_with_asan && echo yes || echo no) -IS_BUILT_WITH_COVERAGE=$(meson configure "${BUILD_DIR:?}" | grep 'b_coverage' | awk '{ print $2 }' | grep -q 'true' && echo yes || echo no) +IS_BUILT_WITH_COVERAGE=$(is_built_with_coverage && echo yes || echo no) if get_bool "$IS_BUILT_WITH_ASAN"; then STRIP_BINARIES=no