| Message ID | 20260127164058.1844919-1-aconole@redhat.com |
|---|---|
| State | Deferred |
| Delegated to: | Eelco Chaudron |
| Headers | show |
| Series | [ovs-dev] github: Add a job to run the code coverage. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/cirrus-robot | success | cirrus build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 27 Jan 2026, at 17:40, Aaron Conole via dev wrote: > While adding new code, we can sometimes miss whether there is sufficient > test case coverage that accommodates the new functionality. Additionally, > code that lacks sufficient test case coverage is ripe for new > contributors to find ways of delivering meaningful contributions to the > code base. > > Add a new CI job that executes the tests with code coverage and archives > the results. To help facilitate this, update the coverage check to also > add '-fprofile-update=atomic' (due to some code paths being > multi-threaded) and make the github job explicitly not optimize code. > The resulting jobs can be analyzed for more details offline due to the > archives. > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- Hi Aaron, Thanks for working on this. I’m a big fan of adding coverage testing, but I want to make sure we get the maximum benefit from the extra CI time this will consume. My main concern is that without a pass/fail threshold, the data might be ignored. Do you have a vision for how contributors would use these logs effectively, especially given GitHub's limited artifact retention? It would be nice if we could eventually get to a "Diff Coverage" report (highlighting only the new code), though I know that is technically complex with our current diff/patching setup (similar to clang-analyze). For now, do you think we should hold off on merging this until we can add some enforcement or clearer reporting? I’m worried about increasing the CI load without a guaranteed actionable outcome. //Eelco
Eelco Chaudron <echaudro@redhat.com> writes: > On 27 Jan 2026, at 17:40, Aaron Conole via dev wrote: > >> While adding new code, we can sometimes miss whether there is sufficient >> test case coverage that accommodates the new functionality. Additionally, >> code that lacks sufficient test case coverage is ripe for new >> contributors to find ways of delivering meaningful contributions to the >> code base. >> >> Add a new CI job that executes the tests with code coverage and archives >> the results. To help facilitate this, update the coverage check to also >> add '-fprofile-update=atomic' (due to some code paths being >> multi-threaded) and make the github job explicitly not optimize code. >> The resulting jobs can be analyzed for more details offline due to the >> archives. >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- > > Hi Aaron, > > Thanks for working on this. I’m a big fan of adding coverage testing, > but I want to make sure we get the maximum benefit from the extra CI > time this will consume. > > My main concern is that without a pass/fail threshold, the data might > be ignored. Do you have a vision for how contributors would use these > logs effectively, especially given GitHub's limited artifact > retention? The idea would be to generate a report that we could use to understand whether code that was modified actually got tested. > It would be nice if we could eventually get to a "Diff Coverage" > report (highlighting only the new code), though I know that is > technically complex with our current diff/patching setup (similar to > clang-analyze). Yes, but that's the goal for the end. > For now, do you think we should hold off on merging this until we can > add some enforcement or clearer reporting? I’m worried about > increasing the CI load without a guaranteed actionable outcome. Okay by me. I don't think it should increase the CI load so much, but I guess it can make sense to shelve this work. > //Eelco
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 96a503ca55..7e46cdb617 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -353,6 +353,90 @@ jobs: name: logs-linux-${{ join(matrix.*, '-') }} path: logs.tgz + build-coverage: + env: + dependencies: | + automake libtool gcc libssl-dev libunbound-dev libunwind-dev lcov + CC: gcc + name: coverage + runs-on: ubuntu-24.04 + timeout-minutes: 60 + + steps: + - name: checkout + uses: actions/checkout@v4 + + - name: update PATH + run: | + echo "$HOME/bin" >> $GITHUB_PATH + echo "$HOME/.local/bin" >> $GITHUB_PATH + + - name: set up python + uses: actions/setup-python@v5 + with: + python-version: ${{ env.python_default }} + + - name: update APT cache + run: sudo apt update || true + + - name: install dependencies + run: sudo apt install -y ${{ env.dependencies }} + + - name: prepare + run: ./.ci/linux-prepare.sh + + - name: configure with coverage + run: | + # Enabling coverage should automatically remove the -O2, so explicitly + # add the -g -O0 + ./boot.sh + ./configure --enable-coverage CFLAGS="-g -O0" + + - name: build + run: make -j$(nproc) + + - name: run tests with coverage + run: make check-lcov TESTSUITEFLAGS="-j$(nproc)" RECHECK=yes + + - name: generate coverage summary + run: | + # Extract coverage summary for GitHub Actions + SUMMARY=$(lcov --summary tests/lcov/coverage.info 2>&1) + LINES=$(echo "$SUMMARY" | grep 'lines' | sed 's/.*: //' | cut -d' ' -f1) + FUNCTIONS=$(echo "$SUMMARY" | grep 'functions' | sed 's/.*: //' | cut -d' ' -f1) + BRANCHES=$(echo "$SUMMARY" | grep 'branches' | sed 's/.*: //' | cut -d' ' -f1) + + echo "## Code Coverage Report" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "| Metric | Coverage |" >> $GITHUB_STEP_SUMMARY + echo "|--------|----------|" >> $GITHUB_STEP_SUMMARY + echo "| Lines | ${LINES:-N/A} |" >> $GITHUB_STEP_SUMMARY + echo "| Functions | ${FUNCTIONS:-N/A} |" >> $GITHUB_STEP_SUMMARY + echo "| Branches | ${BRANCHES:-N/A} |" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "Download the coverage artifact below to view the full HTML report." >> $GITHUB_STEP_SUMMARY + + - name: upload coverage report + uses: actions/upload-artifact@v4 + with: + name: coverage-report + path: tests/lcov/ + + - name: copy logs on failure + if: failure() || cancelled() + run: | + mkdir logs + cp config.log ./logs/ || true + cp -r ./tests/testsuite.* ./logs/ || true + sudo tar -czvf logs.tgz logs/ + + - name: upload logs on failure + if: failure() || cancelled() + uses: actions/upload-artifact@v4 + with: + name: logs-coverage + path: logs.tgz + build-clang-analyze-cache: needs: build-dpdk env: diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4 index 0600704759..7902b1f1ad 100644 --- a/m4/openvswitch.m4 +++ b/m4/openvswitch.m4 @@ -42,8 +42,8 @@ AC_DEFUN([OVS_CHECK_COVERAGE], esac done - OVS_CFLAGS="$OVS_CFLAGS --coverage" - OVS_LDFLAGS="$OVS_LDFLAGS --coverage" + OVS_CFLAGS="$OVS_CFLAGS --coverage -fprofile-update=atomic" + OVS_LDFLAGS="$OVS_LDFLAGS --coverage -fprofile-update=atomic" fi]) dnl Checks for --enable-ndebug and defines NDEBUG if it is specified.
While adding new code, we can sometimes miss whether there is sufficient test case coverage that accommodates the new functionality. Additionally, code that lacks sufficient test case coverage is ripe for new contributors to find ways of delivering meaningful contributions to the code base. Add a new CI job that executes the tests with code coverage and archives the results. To help facilitate this, update the coverage check to also add '-fprofile-update=atomic' (due to some code paths being multi-threaded) and make the github job explicitly not optimize code. The resulting jobs can be analyzed for more details offline due to the archives. Signed-off-by: Aaron Conole <aconole@redhat.com> --- .github/workflows/build-and-test.yml | 84 ++++++++++++++++++++++++++++ m4/openvswitch.m4 | 4 +- 2 files changed, 86 insertions(+), 2 deletions(-)