diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1d02cea12..8b66271953 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -164,9 +164,9 @@ Regardless of the type of PR, all PRs should include: * well documented code changes. * additional testcases. Ideally, they should fail w/o your code change applied. (With a few exceptions, CI hooks will block your PR unless your change - includes files named `*_test.go` or under the `test/` subdirectory. To - bypass this block, include the string `[NO NEW TESTS NEEDED]` in your - commit message). + includes files named `*_test.go` or under the `test/` subdirectory. Repo + admins may bypass this restriction by setting the 'No New Tests' GitHub + label on the PR). * documentation changes. Squash your commits into logical pieces of work that might want to be reviewed diff --git a/contrib/cirrus/pr-should-include-tests b/contrib/cirrus/pr-should-include-tests index 7c76cb90fa..9e41de87d2 100755 --- a/contrib/cirrus/pr-should-include-tests +++ b/contrib/cirrus/pr-should-include-tests @@ -2,17 +2,16 @@ # # Intended for use in CI: check git commits, barf if no tests added. # +ME=$(basename $0) + +# Github label which allows overriding this check +OVERRIDE_LABEL="No New Tests" # Docs-only changes are excused if [[ "${CIRRUS_CHANGE_TITLE}" =~ CI:DOCS ]]; then exit 0 fi -# So are PRs where 'NO NEW TESTS NEEDED' appears in the Github message -if [[ "${CIRRUS_CHANGE_MESSAGE}" =~ NO.NEW.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. @@ -51,14 +50,41 @@ if [[ -z "$filtered_changes" ]]; then exit 0 fi -# One last chance: perhaps the developer included the magic '[NO NEW TESTS NEEDED]' -# string in an amended commit. -if git log --format=%B ${base}..${head} | grep -F '[NO NEW TESTS NEEDED]'; then - exit 0 +# Nope. Only allow if the github 'no-tests-needed' label is set +if [[ -z "$CIRRUS_PR" ]]; then + echo "$ME: cannot query github: \$CIRRUS_PR is undefined" >&2 + exit 1 +fi +if [[ -z "$CIRRUS_REPO_CLONE_TOKEN" ]]; then + echo "$ME: cannot query github: \$CIRRUS_REPO_CLONE_TOKEN is undefined" >&2 + exit 1 +fi + +query="{ + \"query\": \"query { + repository(owner: \\\"containers\\\", name: \\\"podman\\\") { + pullRequest(number: $CIRRUS_PR) { + labels(first: 100) { + nodes { + name + } + } + } + } +}\" +}" + +result=$(curl -s -H "Authorization: bearer $CIRRUS_REPO_CLONE_TOKEN" -H "Accept: application/vnd.github.antiope-preview+json" -H "Content-Type: application/json" -X POST --data @- https://api.github.com/graphql <<<"$query") + +labels=$(jq -r '.data.repository.pullRequest.labels.nodes[].name' <<<"$result") + +if grep -F -x -q "$OVERRIDE_LABEL" <<<"$labels"; then + # PR has the label set + exit 0 fi cat <&2 + exit 1 +fi +export CIRRUS_REPO_CLONE_TOKEN="$GITHUB_TOKEN" + ############################################################################### # BEGIN test cases # @@ -26,19 +33,19 @@ ME=$(basename $0) # 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 8f75ed958 7b3ad6d89 PR 8835, only a README.md change -0 b6db60e58 f06dd45e0 PR 9420, a test rename -0 c6a896b0c 4ea5d6971 PR 11833, includes magic string +0 68c9e02df db71759b1 8821 multiple commits, includes tests +0 bb82c37b7 eeb4c129b 8832 single commit, w/tests, merge-base test +1 1f5927699 864592c74 8685 multiple commits, no tests +0 7592f8fbb 6bbe54f2b 8766 no tests, but CI:DOCS in commit message +0 355e38769 bfbd915d6 8884 a vendor bump +0 ffe2b1e95 e467400eb 8899 only .cirrus.yml +0 06a6fd9f2 3cc080151 8695 docs-only, without CI:DOCS +0 a47515008 ecedda63a 8816 unit tests only +0 caa84cd35 e55320efd 8565 hack/podman-socat only +0 c342583da 12f835d12 8523 version.go + podman.spec.in +0 8f75ed958 7b3ad6d89 8835 only a README.md change +0 b6db60e58 f06dd45e0 9420 a test rename +0 c6a896b0c 4ea5d6971 11833 includes magic string " # The script we're testing @@ -76,10 +83,10 @@ function run_test_script() { echo "# Actual: $output" rc=1 else - echo "ok $testnum $testname" + echo "ok $testnum $testname - rc=$expected_rc" fi else - echo "ok $testnum $testname" + echo "ok $testnum $testname - rc=$expected_rc" fi fi @@ -97,15 +104,6 @@ function run_test_script() { 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 @@ -119,7 +117,7 @@ rc=0 testnum=0 tested_override= -while read expected_rc parent_sha commit_sha rest; do +while read expected_rc parent_sha commit_sha pr rest; do # Skip blank lines test -z "$expected_rc" && continue @@ -127,8 +125,9 @@ while read expected_rc parent_sha commit_sha rest; do export CIRRUS_CHANGE_IN_REPO=$commit_sha export CIRRUS_CHANGE_TITLE=$(git log -1 --format=%s $commit_sha) export CIRRUS_CHANGE_MESSAGE= + export CIRRUS_PR=$pr - run_test_script $expected_rc "$rest" + run_test_script $expected_rc "PR $pr - $rest" done <<<"$tests" echo "1..$testnum"