From 7d6a59ba77bce90e7bdaf6746feca5772c805195 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Wed, 1 Feb 2023 23:05:00 +0000 Subject: [PATCH] test: Update coverage tests to Go 1.20 Go 1.20 finally includes proper support for instrumenting binaries for coverage. This allows us to drop quite a few hacks and workarounds that we used for it, and we can now also test exiting cases. The downside is that coverage tests now require Go 1.20, but it is an acceptable price to pay for the more accurate results. Normal integration tests are unchanged. This patch updates the coverage testing infrastructure to make use of the new Go 1.20 features. --- chasquid.go | 4 +- cmd/dovecot-auth-cli/coverage_test.go | 34 ---------- cmd/dovecot-auth-cli/coverage_wrapper | 7 -- cmd/dovecot-auth-cli/test.sh | 12 ++-- coverage_test.go | 40 ------------ test/cover.sh | 35 +++++----- test/util/gocovcat/gocovcat.go | 92 --------------------------- test/util/lib.sh | 25 +------- 8 files changed, 25 insertions(+), 224 deletions(-) delete mode 100644 cmd/dovecot-auth-cli/coverage_test.go delete mode 100755 cmd/dovecot-auth-cli/coverage_wrapper delete mode 100644 coverage_test.go delete mode 100644 test/util/gocovcat/gocovcat.go diff --git a/chasquid.go b/chasquid.go index 18067ce..fbf0274 100644 --- a/chasquid.go +++ b/chasquid.go @@ -235,7 +235,7 @@ func signalHandler() { var err error signals := make(chan os.Signal, 1) - signal.Notify(signals, syscall.SIGHUP) + signal.Notify(signals, syscall.SIGHUP, syscall.SIGTERM, syscall.SIGINT) for { switch sig := <-signals; sig { @@ -251,6 +251,8 @@ func signalHandler() { if err != nil { log.Fatalf("Error reopening maillog: %v", err) } + case syscall.SIGTERM, syscall.SIGINT: + log.Fatalf("Got signal to exit: %v", sig) default: log.Errorf("Unexpected signal %v", sig) } diff --git a/cmd/dovecot-auth-cli/coverage_test.go b/cmd/dovecot-auth-cli/coverage_test.go deleted file mode 100644 index 97c80db..0000000 --- a/cmd/dovecot-auth-cli/coverage_test.go +++ /dev/null @@ -1,34 +0,0 @@ -// This package is tested externally (see test.sh). -// However, we need this to do coverage tests. -// -// See coverage_test.go for the details, this is the same horrible hack. -// -//go:build coveragebin -// +build coveragebin - -package main - -import ( - "os" - "os/signal" - "syscall" - "testing" -) - -func TestRunMain(t *testing.T) { - done := make(chan bool) - - signals := make(chan os.Signal, 1) - go func() { - <-signals - done <- true - }() - signal.Notify(signals, os.Interrupt, os.Kill, syscall.SIGTERM) - - go func() { - main() - done <- true - }() - - <-done -} diff --git a/cmd/dovecot-auth-cli/coverage_wrapper b/cmd/dovecot-auth-cli/coverage_wrapper deleted file mode 100755 index d2188e4..0000000 --- a/cmd/dovecot-auth-cli/coverage_wrapper +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/bash -# -# Wrapper for dovecot-auth-cli to run when we build it in coverage mode. - -exec ./dovecot-auth-cli.test -test.run ^TestRunMain$ \ - -test.coverprofile="$COVER_DIR/test-`date +%s.%N`.out" \ - "$@" diff --git a/cmd/dovecot-auth-cli/test.sh b/cmd/dovecot-auth-cli/test.sh index f837946..e5e82a9 100755 --- a/cmd/dovecot-auth-cli/test.sh +++ b/cmd/dovecot-auth-cli/test.sh @@ -8,15 +8,13 @@ init # Build the binary once, so we can use it and launch it in chamuyero scripts. # Otherwise, we not only spend time rebuilding it over and over, but also "go # run" masks the exit code, which is something we care about. -# shellcheck disable=SC2086 -if [ "${COVER_DIR}" != "" ]; then - go test -covermode=count -coverpkg=../../... -c \ - $GOFLAGS -tags="coveragebin $GOTAGS" - cp coverage_wrapper dovecot-auth-cli -else - go build $GOFLAGS -tags="$GOTAGS" dovecot-auth-cli.go +if [ "${GOCOVERDIR}" != "" ]; then + GOFLAGS="-cover -covermode=count -o dovecot-auth-cli $GOFLAGS" fi +# shellcheck disable=SC2086 +go build $GOFLAGS -tags="$GOTAGS" . + if ! ./dovecot-auth-cli lalala 2>&1 | grep -q "invalid arguments"; then echo "cli worked with invalid arguments" exit 1 diff --git a/coverage_test.go b/coverage_test.go deleted file mode 100644 index ae30c37..0000000 --- a/coverage_test.go +++ /dev/null @@ -1,40 +0,0 @@ -// Test file used to build a coverage-enabled chasquid binary. -// -// Go lacks support for properly building a coverage binary, it can only build -// coverage test binaries. As a workaround, we have a test that just runs -// main. We then build a binary of this test, which we use instead of chasquid -// in integration tests. -// -// This is hacky and horrible. -// -// The test has a build label so it's not accidentally executed during normal -// "go test" invocations. -//go:build coveragebin -// +build coveragebin - -package main - -import ( - "os" - "os/signal" - "syscall" - "testing" -) - -func TestRunMain(t *testing.T) { - done := make(chan bool) - - signals := make(chan os.Signal, 1) - go func() { - <-signals - done <- true - }() - signal.Notify(signals, os.Interrupt, os.Kill, syscall.SIGTERM) - - go func() { - main() - done <- true - }() - - <-done -} diff --git a/test/cover.sh b/test/cover.sh index adccaaa..3f2e874 100755 --- a/test/cover.sh +++ b/test/cover.sh @@ -19,41 +19,36 @@ cd "${TBASE}/.." # Recreate the coverage output directory, to avoid including stale results # from previous runs. rm -rf .coverage -mkdir -p .coverage +mkdir -p .coverage/sh .coverage/go .coverage/all export COVER_DIR="$PWD/.coverage" # Normal go tests. -# We have to run them one by one because the expvar registration causes -# the single-binary tests to fail: cross-package expvars confuse the expvarom -# tests, which don't expect any expvars to exists besides the one registered -# in the tests themselves. -for pkg in $(go list ./... | grep -v -E 'chasquid/cmd/|chasquid/test'); do - OUT_FILE="$COVER_DIR/pkg-${pkg//\//_}.out" - go test -tags coverage \ - -covermode=count \ - -coverprofile="$OUT_FILE" \ - -coverpkg=./... "$pkg" -done +# shellcheck disable=SC2046 +go test \ + -covermode=count -coverpkg=./... \ + $(go list ./... | grep -v -E 'chasquid/cmd/|chasquid/test') \ + -args -test.gocoverdir="${COVER_DIR}/go/" # Integration tests. # Will run in coverage mode due to $COVER_DIR being set. -setsid -w ./test/run.sh +GOCOVERDIR="${COVER_DIR}/sh" setsid -w ./test/run.sh # dovecot tests are also coverage-aware. echo "dovecot cli ..." -setsid -w ./cmd/dovecot-auth-cli/test.sh +GOCOVERDIR="${COVER_DIR}/sh" setsid -w ./cmd/dovecot-auth-cli/test.sh # Merge all coverage output into a single file. +go tool covdata merge -i "${COVER_DIR}/go,${COVER_DIR}/sh" -o "${COVER_DIR}/all" +go tool covdata textfmt -i "${COVER_DIR}/all" -o "${COVER_DIR}/merged.out" + # Ignore protocol buffer-generated files, as they are not relevant. -go run "${UTILDIR}/gocovcat/gocovcat.go" .coverage/*.out \ - | grep -v ".pb.go:" \ - > .coverage/all.out +grep -v ".pb.go:" < "${COVER_DIR}/merged.out" > "${COVER_DIR}/final.out" # Generate reports based on the merged output. -go tool cover -func="$COVER_DIR/all.out" | sort -k 3 -n > "$COVER_DIR/func.txt" -go tool cover -html="$COVER_DIR/all.out" -o "$COVER_DIR/classic.html" +go tool cover -func="$COVER_DIR/final.out" | sort -k 3 -n > "$COVER_DIR/func.txt" +go tool cover -html="$COVER_DIR/final.out" -o "$COVER_DIR/classic.html" go run "${UTILDIR}/coverhtml/coverhtml.go" \ - -input="$COVER_DIR/all.out" -strip=3 \ + -input="$COVER_DIR/final.out" -strip=3 \ -output="$COVER_DIR/coverage.html" \ -title="chasquid coverage report" \ -notes="Generated at commit $(git describe --always --dirty --tags) ($(git log -1 --format=%ci))" diff --git a/test/util/gocovcat/gocovcat.go b/test/util/gocovcat/gocovcat.go deleted file mode 100644 index 23cee08..0000000 --- a/test/util/gocovcat/gocovcat.go +++ /dev/null @@ -1,92 +0,0 @@ -//usr/bin/env go run "$0" "$@"; exit $? -// -// From: https://git.lukeshu.com/go/cmd/gocovcat/ -// -//go:build !coverage -// +build !coverage - -// Copyright 2017 Luke Shumaker -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -// Command gocovcat combines multiple go cover runs, and prints the -// result on stdout. -package main - -import ( - "bufio" - "fmt" - "os" - "sort" - "strconv" - "strings" -) - -func handleErr(err error) { - if err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } -} - -func main() { - modeBool := false - blocks := map[string]int{} - for _, filename := range os.Args[1:] { - file, err := os.Open(filename) - handleErr(err) - buf := bufio.NewScanner(file) - for buf.Scan() { - line := buf.Text() - - if strings.HasPrefix(line, "mode: ") { - m := strings.TrimPrefix(line, "mode: ") - switch m { - case "set": - modeBool = true - case "count", "atomic": - // do nothing - default: - fmt.Fprintf(os.Stderr, "Unrecognized mode: %s\n", m) - os.Exit(1) - } - } else { - sp := strings.LastIndexByte(line, ' ') - block := line[:sp] - cntStr := line[sp+1:] - cnt, err := strconv.Atoi(cntStr) - handleErr(err) - blocks[block] += cnt - } - } - handleErr(buf.Err()) - } - keys := make([]string, 0, len(blocks)) - for key := range blocks { - keys = append(keys, key) - } - sort.Strings(keys) - modeStr := "count" - if modeBool { - modeStr = "set" - } - fmt.Printf("mode: %s\n", modeStr) - for _, block := range keys { - cnt := blocks[block] - if modeBool && cnt > 1 { - cnt = 1 - } - fmt.Printf("%s %d\n", block, cnt) - } -} diff --git a/test/util/lib.sh b/test/util/lib.sh index 9b6aa2b..008ea0e 100644 --- a/test/util/lib.sh +++ b/test/util/lib.sh @@ -27,9 +27,8 @@ function init() { } function chasquid() { - if [ "${COVER_DIR}" != "" ]; then - chasquid_cover "$@" - return + if [ "${GOCOVERDIR}" != "" ]; then + GOFLAGS="-cover -covermode=count -o chasquid $GOFLAGS" fi # shellcheck disable=SC2086 @@ -44,26 +43,6 @@ function chasquid() { "${TBASE}/../../chasquid" "$@" } -function chasquid_cover() { - # Build the coverage-enabled binary. - # See coverage_test.go for more details. - # shellcheck disable=SC2086 - ( cd "${TBASE}/../../" || exit 1; - go test -covermode=count -coverpkg=./... -c \ - -tags="coveragebin $GOTAGS" $GOFLAGS ) - - # Run the coverage-enabled binary, named "chasquid.test" for hacky - # reasons. See the chasquid function above for details on the - # environment variables. - HOSTALIASES=${TBASE}/hosts \ - PATH=${UTILDIR}:${PATH} \ - MDA_DIR=${TBASE}/.mail \ - "${TBASE}/../../chasquid.test" \ - -test.run "^TestRunMain$" \ - -test.coverprofile="$COVER_DIR/test-$(date +%s.%N).out" \ - "$@" -} - # Add a user with chasquid-util. Because this is somewhat cryptographically # intensive, it can slow down the tests significantly, so most of the time we # use the simpler add_user (below) for testing purposes.