summaryrefslogtreecommitdiff
path: root/contrib
diff options
context:
space:
mode:
authorEd Santiago <santiago@redhat.com>2021-01-07 13:57:35 -0700
committerEd Santiago <santiago@redhat.com>2021-01-19 10:45:58 -0700
commit9f6bb3563d7a311a7e5628d18210c2c263af962d (patch)
tree9d208c129ff8ea39a0c2c0057b153896824ed949 /contrib
parent8c6df5e93e6941a7f50da651678751f7dfec900e (diff)
downloadpodman-9f6bb3563d7a311a7e5628d18210c2c263af962d.tar.gz
podman-9f6bb3563d7a311a7e5628d18210c2c263af962d.tar.bz2
podman-9f6bb3563d7a311a7e5628d18210c2c263af962d.zip
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 <santiago@redhat.com>
Diffstat (limited to 'contrib')
-rwxr-xr-xcontrib/cirrus/pr-should-include-tests70
-rwxr-xr-xcontrib/cirrus/pr-should-include-tests.t137
-rwxr-xr-xcontrib/cirrus/runner.sh4
3 files changed, 211 insertions, 0 deletions
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 <<EOF
+$(basename $0): PR does not include changes in the 'tests' directory
+
+Please write a regression test for what you're fixing. Even if it
+seems trivial or obvious, try to add a test that will prevent
+regressions.
+
+If your change is minor, feel free to piggyback on already-written
+tests, possibly just adding a small step to a similar existing test.
+Every second counts in CI.
+
+If your commit really, truly does not need tests, you can proceed
+by adding '[NO TESTS NEEDED]' to the body of your commit message.
+Please think carefully before doing so.
+EOF
+
+exit 1
diff --git a/contrib/cirrus/pr-should-include-tests.t b/contrib/cirrus/pr-should-include-tests.t
new file mode 100755
index 000000000..4618060a3
--- /dev/null
+++ b/contrib/cirrus/pr-should-include-tests.t
@@ -0,0 +1,137 @@
+#!/bin/bash
+#
+# tests for pr-should-include-tests.t
+#
+# FIXME: I don't think this will work in CI, because IIRC the git-checkout
+# is a shallow one. But it works fine in a developer tree.
+#
+ME=$(basename $0)
+
+###############################################################################
+# BEGIN test cases
+#
+# Feel free to add as needed. Syntax is:
+# <exit status> <sha of commit> <branch>=<sha of merge base> # 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
}