From 9f6bb3563d7a311a7e5628d18210c2c263af962d Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Thu, 7 Jan 2021 13:57:35 -0700 Subject: CI: smoke test: insist on adding tests on PRs On each PR (with a few exceptions), check the list of git-touched files, and abort if no tests are added. Include instructions on how to bypass the check if tests really aren't needed. Include a hardcoded exception list for PRs that only touch a well-known subset of "safe" files: docs, .cirrus.yml, vendor, version, hack, contrib, or *.md. This list is likely to need tuning over time. Add a test suite, but not one recognized by the new script (because it's a "*.t" file), so: [NO TESTS NEEDED] Signed-off-by: Ed Santiago --- contrib/cirrus/pr-should-include-tests | 70 ++++++++++++++++ contrib/cirrus/pr-should-include-tests.t | 137 +++++++++++++++++++++++++++++++ contrib/cirrus/runner.sh | 4 + 3 files changed, 211 insertions(+) create mode 100755 contrib/cirrus/pr-should-include-tests create mode 100755 contrib/cirrus/pr-should-include-tests.t (limited to 'contrib') diff --git a/contrib/cirrus/pr-should-include-tests b/contrib/cirrus/pr-should-include-tests new file mode 100755 index 000000000..caf27cf83 --- /dev/null +++ b/contrib/cirrus/pr-should-include-tests @@ -0,0 +1,70 @@ +#!/bin/bash +# +# Intended for use in CI: check git commits, barf if no tests added. +# + +# Docs-only changes are excused +if [[ "${CIRRUS_CHANGE_TITLE}" =~ CI:DOCS ]]; then + exit 0 +fi + +# So are PRs where 'NO TESTS NEEDED' appears in the Github message +if [[ "${CIRRUS_CHANGE_MESSAGE}" =~ NO.TESTS.NEEDED ]]; then + exit 0 +fi + +# HEAD should be good enough, but the CIRRUS envariable allows us to test +head=${CIRRUS_CHANGE_IN_REPO:-HEAD} +# Base of this PR. Here we absolutely rely on cirrus. +base=$(git merge-base ${DEST_BRANCH:-master} $head) + +# This gives us a list of files touched in all commits, e.g. +# A foo.c +# M bar.c +# We look for Added or Modified (not Deleted!) files under 'test'. +if git diff --name-status $base $head | egrep -q '^[AM]\s+(test/|.*_test\.go)'; then + exit 0 +fi + +# Nothing changed under test subdirectory. +# +# This is OK if the only files being touched are "safe" ones. +filtered_changes=$(git diff --name-status $base $head | + awk '{print $2}' | + fgrep -vx .cirrus.yml | + fgrep -vx changelog.txt | + fgrep -vx go.mod | + fgrep -vx go.sum | + egrep -v '^[^/]+\.md$' | + egrep -v '^contrib/' | + egrep -v '^docs/' | + egrep -v '^hack/' | + egrep -v '^vendor/' | + egrep -v '^version/') +if [[ -z "$filtered_changes" ]]; then + exit 0 +fi + +# One last chance: perhaps the developer included the magic '[NO TESTS NEEDED]' +# string in an amended commit. +if git log --format=%B ${base}..${head} | fgrep '[NO TESTS NEEDED]'; then + exit 0 +fi + +cat < = # comments +# +# Where: +# exit status is the expected exit status of the script +# sha of merge base is the SHA of the branch point of the commit +# sha of commit is the SHA of a real commit in the podman repo +# +# We need the actual sha of the merge base because once a branch is +# merged 'git merge-base' (used in our test script) becomes useless. +# +# +# FIXME: as of 2021-01-07 we don't have "no tests needed" in our git +# commit history, but once we do, please add a new '0' test here. +# +tests=" +0 68c9e02df db71759b1 PR 8821: multiple commits, includes tests +0 bb82c37b7 eeb4c129b PR 8832: single commit, w/tests, merge-base test +1 1f5927699 864592c74 PR 8685, multiple commits, no tests +0 7592f8fbb 6bbe54f2b PR 8766, no tests, but CI:DOCS in commit message +0 355e38769 bfbd915d6 PR 8884, a vendor bump +0 ffe2b1e95 e467400eb PR 8899, only .cirrus.yml +0 06a6fd9f2 3cc080151 PR 8695, docs-only, without CI:DOCS +0 a47515008 ecedda63a PR 8816, unit tests only +0 caa84cd35 e55320efd PR 8565, hack/podman-socat only +0 c342583da 12f835d12 PR 8523, version.go + podman.spec.in +0 c342583da db1d2ff11 version bump to v2.2.0 +0 8f75ed958 7b3ad6d89 PR 8835, only a README.md change +" + +# The script we're testing +test_script=$(dirname $0)/$(basename $0 .t) + +# END test cases +############################################################################### +# BEGIN test-script runner and status checker + +function run_test_script() { + local expected_rc=$1 + local testname=$2 + + testnum=$(( testnum + 1 )) + + # DO NOT COMBINE 'local output=...' INTO ONE LINE. If you do, you lose $? + local output + output=$( $test_script ) + local actual_rc=$? + + if [[ $actual_rc != $expected_rc ]]; then + echo "not ok $testnum $testname" + echo "# expected rc $expected_rc" + echo "# actual rc $actual_rc" + if [[ -n "$output" ]]; then + echo "# script output: $output" + fi + rc=1 + else + if [[ $expected_rc == 1 ]]; then + # Confirm we get an error message + if [[ ! "$output" =~ "Please write a regression test" ]]; then + echo "not ok $testnum $testname" + echo "# Expected: ~ 'Please write a regression test'" + echo "# Actual: $output" + rc=1 + else + echo "ok $testnum $testname" + fi + else + echo "ok $testnum $testname" + fi + fi + + # If we expect an error, confirm that we can override it. We only need + # to do this once. + if [[ $expected_rc == 1 ]]; then + if [[ -z "$tested_override" ]]; then + testnum=$(( testnum + 1 )) + + CIRRUS_CHANGE_TITLE="[CI:DOCS] hi there" $test_script &>/dev/null + if [[ $? -ne 0 ]]; then + echo "not ok $testnum $rest (override with CI:DOCS)" + rc=1 + else + echo "ok $testnum $rest (override with CI:DOCS)" + fi + + testnum=$(( testnum + 1 )) + CIRRUS_CHANGE_MESSAGE="hi there [NO TESTS NEEDED] bye" $test_script &>/dev/null + if [[ $? -ne 0 ]]; then + echo "not ok $testnum $rest (override with '[NO TESTS NEEDED]')" + rc=1 + else + echo "ok $testnum $rest (override with '[NO TESTS NEEDED]')" + fi + + tested_override=1 + fi + fi +} + +# END test-script runner and status checker +############################################################################### +# BEGIN test-case parsing + +rc=0 +testnum=0 +tested_override= + +while read expected_rc parent_sha commit_sha rest; do + # Skip blank lines + test -z "$expected_rc" && continue + + export DEST_BRANCH=$parent_sha + export CIRRUS_CHANGE_IN_REPO=$commit_sha + export CIRRUS_CHANGE_TITLE=$(git log -1 --format=%s $commit_sha) + export CIRRUS_CHANGE_MESSAGE= + + run_test_script $expected_rc "$rest" +done <<<"$tests" + +echo "1..$testnum" +exit $rc + +# END Test-case parsing +############################################################################### diff --git a/contrib/cirrus/runner.sh b/contrib/cirrus/runner.sh index b0060163e..e08bb5f19 100755 --- a/contrib/cirrus/runner.sh +++ b/contrib/cirrus/runner.sh @@ -31,7 +31,11 @@ function _run_smoke() { # $CIRRUS_TAG is only non-empty when executing due to a tag-push # shellcheck disable=SC2154 if [[ -z "$CIRRUS_TAG" ]]; then + # If PR consists of multiple commits, test that each compiles cleanly make .gitvalidation + + # PRs should include some way to test. + $SCRIPT_BASE/pr-should-include-tests fi } -- cgit v1.2.3-54-g00ecf