Coverage - 2nd attempt

.github/workflows/tests.yml

@@ -59,7 +59,10 @@ jobs:
               --classic
 
       - name: Build snap
-        run: make $MICROOVN_SNAP
+        run: |
+          # Build snap with coverage support
+          sed -i 's/MICROOVN_COVERAGE=.*/MICROOVN_COVERAGE="yes"/g' microovn/build-aux/environment
+          make $MICROOVN_SNAP
 
       - name: Upload artifacts
         if: always()
@@ -132,4 +135,60 @@ jobs:
           snap list
 
       - name: Run system tests
-        run: .bats/bats-core/bin/bats tests/${{ matrix.test-file }}
+        run: MICROOVN_COVERAGE_ENABLED=yes make tests/${{ matrix.test-file }}
+
+      - name: Upload test coverage
+        uses: actions/upload-artifact@v4
+        with:
+          name: ${{ matrix.test-file }}_coverage
+          path: ${{ github.workspace }}/.coverage
+          include-hidden-files: true
+          retention-days: 1
+
+  generate-coverage:
+    name: Generate coverage profile
+    needs:
+      - metadata
+      - system-tests
+    # 'ubuntu-latest' currently resolves to '22.04' [0] and since we require "Go >=1.20"
+    # for coverage tools, we need to use explicit 'ubuntu-24.04' image name.
+    # [0] https://github.com/actions/runner-images/issues/10636
+    runs-on: ubuntu-24.04
+    env:
+      COVERAGE_DIR: ${{ github.workspace }}/.coverage
+      COVERAGE_MERGED: ${{ github.workspace }}/.coverage/_merged
+      COVERAGE_MERGED_PROFILE: ${{ github.workspace }}/.coverage/_merged/profile.out
+    steps:
+      - name: Install dependencies
+        run: |
+          sudo apt install -yqq golang
+          go install github.com/boumenot/[email protected]
+
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Download test coverage data
+        uses: actions/download-artifact@v4
+        with:
+          path: ${{ env.COVERAGE_DIR }}
+          pattern: "*_coverage"
+
+      - name: Merge test coverage data
+        run: |
+          mkdir -p "$COVERAGE_MERGED"
+          coverage_inputs=$(find "$COVERAGE_DIR" -type d -name coverage | tr '\n' ',' | sed 's/,$//g')
+          go tool covdata merge -i="$coverage_inputs" -o="$COVERAGE_MERGED"
+          go tool covdata textfmt -i="$COVERAGE_MERGED" -o="$COVERAGE_MERGED_PROFILE"
+
+      - name: Generate cobertura.xml
+        run: |
+          cd microovn/
+          $HOME/go/bin/gocover-cobertura < "$COVERAGE_MERGED_PROFILE" > "$COVERAGE_DIR/cobertura.xml"
+
+      - name: Upload cobertura.xml
+        uses: actions/upload-artifact@v4
+        with:
+          name: cobertura.xml
+          path: ${{ env.COVERAGE_DIR }}/cobertura.xml
+          include-hidden-files: true
+

.gitignore

@@ -1 +1,4 @@
 *.snap
+
+# collected coverage data
+.coverage/

Makefile

@@ -8,6 +8,8 @@ MICROOVN_SOURCES := $(shell find microovn/ -type f)
 COMMAND_WRAPPERS := $(shell find snapcraft/ -type f)
 SNAP_SOURCES := $(shell find snap/ -type f)
 
+export MICROOVN_COVERAGE_DST := $(CURDIR)/.coverage
+
 check: check-lint check-system
 
 check-tabs:

docs/developers/building.rst

@@ -38,6 +38,20 @@ To build MicroOVN, go into the repository's root directory and run:
 This will produce the ``microovn.snap`` file that can be then used to install
 MicroOVN on your system.
 
+Adjust build parameters
+-----------------------
+
+``snapcraft.yaml`` is by nature a very static build recipe that does not allow
+build-time modification without changing the file itself. To achieve some
+level of control over MicroOVN builds, we are using a
+``microovn/build-aux/environment`` file that is loaded and during the build
+process. Environment variables defined in this file can influence properties
+of the final build. Currently supported variables are:
+
+* ``MICROOVN_COVERAGE`` (default: ``no``) - When set to ``yes``, MicroOVN binaries
+  will be built with coverage instrumentation and output coverage data into
+  ``$SNAP_COMMON/data/coverage``.
+
 Install MicroOVN
 ----------------
 

docs/developers/testing.rst

@@ -128,6 +128,25 @@ is supported through the use of the ``LXC_REMOTE`` `LXD environment`_ variable.
    To avoid interleaving output from these parallel test suites, you can
    specify the ``-O`` argument as well.
 
+Test coverage information
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When MicroOVN build is configured with the code coverage support via
+``microovn/build-aux/environment`` file (see more information about adjusting
+MicroOVN build parameters in :doc:`Build MicroOVN <building>` page), system
+tests can collect coverage data. All you need to do is export
+``MICROOVN_COVERAGE_ENABLED=yes`` environment variable. Example
+.. code-block:: none
+
+   # Run all test suites with code coverage
+   export MICROOVN_COVERAGE_ENABLED=yes
+   make check-system
+
+You can find collected data in the ``.coverage/`` directory, where it's
+organised in a ``<test_name>/<container_name>/coverage`` structure. For more
+information about the coverage data format and what you can do with it, see
+`Go Coverage Documentation`_.
+
 Clean up
 ~~~~~~~~
 
@@ -155,3 +174,4 @@ Any leftover containers will be named according to:
 .. _LXD remotes: https://documentation.ubuntu.com/lxd/en/latest/remotes/
 .. _LXD environment: https://documentation.ubuntu.com/lxd/en/latest/environment/
 .. _golangci-lint: https://golangci-lint.run/
+.. _Go Coverage Documentation: https://go.dev/doc/build-cover#working

microovn/build-aux/environment

@@ -0,0 +1,4 @@
+# Commiting changes of this file to the repository
+# will adjust MicroOVN build defaults for everyone. It
+# should generally not be done.
+export MICROOVN_COVERAGE="no"

snap/snapcraft.yaml

@@ -256,6 +256,17 @@ parts:
       go get -d -v ./...
     override-build: |
       set -ex
+
+      # Load dynamic configuration options for MicroOVN snap build
+      source ./build-aux/environment
+
+      # Conditionally build MicroOVN with coverage instrumentation
+      EXTRA_BUILD_ARGS=""
+      echo "" > $CRAFT_STAGE/coverage.env
+      if [ "$MICROOVN_COVERAGE" = "yes" ]; then
+        EXTRA_BUILD_ARGS="-cover"
+        cp "$CRAFT_PROJECT_DIR/snapcraft/coverage.env.ignore" "$CRAFT_STAGE/coverage.env"
+      fi
 
       # Setup build environment
       export CGO_CFLAGS="-I${CRAFT_STAGE}/include/ -I${CRAFT_STAGE}/usr/local/include/"
@@ -293,20 +304,23 @@ parts:
       craftctl set version=${ovn_pkg_short_version}+snap${git_version}
 
       version_package=github.com/canonical/microovn/microovn/version
-      go_ldflags="-X '${version_package}.MicroOvnVersion=${git_version}' \
+      go_ldflags="-X '${version_package}.MicroOvnVersion=${git_version}${EXTRA_BUILD_ARGS}' \
                   -X '${version_package}.OvnVersion=${ovn_pkg_version}' \
                   -X '${version_package}.OvsVersion=${ovs_pkg_version}'"
       # Build the binaries
       go build -o "${CRAFT_PART_INSTALL}/bin/microovn" \
                -ldflags "$go_ldflags" \
+               $EXTRA_BUILD_ARGS \
                ./cmd/microovn
       go build -o "${CRAFT_PART_INSTALL}/bin/microovnd" \
                -ldflags "$go_ldflags" \
                -tags=libsqlite3 \
+               $EXTRA_BUILD_ARGS \
                ./cmd/microovnd
     prime:
       - bin/microovn
       - bin/microovnd
+      - coverage.env
 
   docs:
     plugin: nil

snapcraft/commands/daemon.start

@@ -2,4 +2,5 @@
 export DQLITE_SOCKET="@snap.${SNAP_INSTANCE_NAME}.dqlite"
 export OVS_RUNDIR="${SNAP_COMMON}/run/switch/"
 
+. "$SNAP/coverage.env" 2>/dev/null || true
 exec microovnd --verbose --state-dir "${SNAP_COMMON}/state"

snapcraft/commands/microovn

@@ -1,2 +1,4 @@
 #!/bin/sh
+
+. "$SNAP/coverage.env" 2>/dev/null || true
 exec microovn --state-dir "${SNAP_COMMON}/state" "$@"

snapcraft/coverage.env.ignore

@@ -0,0 +1,7 @@
+# This file is optionally included in the build by snapcraft
+# and loaded by MicroOVN binaries to set output directory for
+# coverage data.
+
+GOCOVERDIR="$SNAP_COMMON/data/coverage"
+export GOCOVERDIR
+mkdir -p "$GOCOVERDIR"

tests/scaleup_cluster.bats

@@ -14,6 +14,7 @@ setup() {
 }
 
 teardown() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }
 

tests/test_helper/bats/lifecycle.bats

@@ -13,6 +13,7 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }
 

tests/test_helper/common.bash

@@ -267,3 +267,42 @@ function ping_reap() {
          while kill -0 \$pid; do sleep 0.1;done && \
          cat /tmp/${base_filename}.stdout"
 }
+
+# collect_coverage CONTAINERS
+#
+# NOTE: Coverage data collection is skipped unless environment
+#       variable MICROOVN_COVERAGE_ENABLED  is set to "yes".
+#
+# WARNING: This function restarts microovn.daemon service on each container in
+#          CONTAINERS, to force 'microovnd' process to write out its coverage data.
+#
+# For each container in CONTAINERS, pull runtime coverage data gathered from MicroOVN
+# daemon and client binaries. Coverage data is by default collected
+# in ".coverage/<test_name>/<container_name>". This location can be controlled by
+# environment variable MICROOVN_COVERAGE_DST
+function collect_coverage() {
+    local containers=$*; shift
+
+    if [ "$MICROOVN_COVERAGE_ENABLED" != "yes" ]; then
+        echo "# Skipping coverage data collection" >&3
+        return 0
+    fi
+
+    local test_name=""
+    test_name=$(basename "$BATS_TEST_FILENAME" | cut -f1 -d\.)
+
+    local dst_prefix="$MICROOVN_COVERAGE_DST"
+    if [ -z "$dst_prefix" ]; then
+        dst_prefix=".coverage"
+    fi
+
+    local container
+    for container in $containers; do
+        echo "# ($container) Collecting coverage information" >&3
+
+        lxc_exec "$container" "snap restart microovn.daemon"
+
+        local output_dir="$dst_prefix/$test_name/$container"
+        lxc_pull_dir "$container/var/snap/microovn/common/data/coverage" "$output_dir"
+    done
+}

tests/test_helper/lxd.bash

@@ -137,3 +137,15 @@ function lxc_file_push() {
 
     lxc file push -q "$file_path" "$container_path"
 }
+
+# lxc_pull_dir CONTAINER_PATH DST
+#
+# Copy directory and all its contents from CONTAINER_PATH to DST.
+# CONTAINER_PATH is a source path in form "<container_name>/path/to/file"
+# and DST is local destination path to which files will be copied.
+function lxc_pull_dir() {
+    local container_path=$1; shift
+    local dst=$1; shift
+
+    lxc file pull -q --recursive "$container_path" "$dst"
+}

tests/test_helper/setup_teardown/basic_cluster.bash

@@ -13,5 +13,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }

tests/test_helper/setup_teardown/central_control.bash

@@ -13,5 +13,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }

tests/test_helper/setup_teardown/init_cluster.bash

@@ -29,6 +29,7 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }
 

tests/test_helper/setup_teardown/init_cluster_custom_encap.bash

@@ -42,6 +42,7 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
     delete_lxd_network "br-east-west"
     rm -f "$BATS_TMPDIR/east_west_addrs.txt"

tests/test_helper/setup_teardown/services.bash

@@ -13,5 +13,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }

tests/test_helper/setup_teardown/services_restart.bash

@@ -25,5 +25,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }

tests/test_helper/setup_teardown/tls_cluster.bash

@@ -29,5 +29,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }

tests/test_helper/setup_teardown/upgrade.bash

@@ -63,5 +63,6 @@ setup_file() {
 }
 
 teardown_file() {
+    collect_coverage $TEST_CONTAINERS
     delete_containers $TEST_CONTAINERS
 }