opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-05-17 23:26:08 +08:00
Code Issues Packages Projects Releases Wiki Activity

CI: must-add-tests check: use GH label, not text

Browse Source 
Old way: edit commit message, add magic string, re-push

New way: repo maintainer adds a Github label to PR, hits Rerun

I've looked and looked for the history behind this script
and why I didn't do it this way in the first place. I've
concluded that I just never thought of it.

Signed-off-by: Ed Santiago <santiago@redhat.com>
This commit is contained in:
Ed Santiago
2024-02-28 15:03:07 -07:00
parent 4fdec1a19a
commit 99bb2bfce4
3 changed files with 66 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
CONTRIBUTING.md
View File

@ -164,9 +164,9 @@ Regardless of the type of PR, all PRs should include:
* well documented code changes. * well documented code changes.
* additional testcases. Ideally, they should fail w/o your code change applied. * 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 (With a few exceptions, CI hooks will block your PR unless your change
includes files named `*_test.go` or under the `test/` subdirectory. To includes files named `*_test.go` or under the `test/` subdirectory. Repo
bypass this block, include the string `[NO NEW TESTS NEEDED]` in your admins may bypass this restriction by setting the 'No New Tests' GitHub
commit message). label on the PR).
* documentation changes. * documentation changes.
Squash your commits into logical pieces of work that might want to be reviewed Squash your commits into logical pieces of work that might want to be reviewed

50
contrib/cirrus/pr-should-include-tests
View File

@ -2,17 +2,16 @@
# #
# Intended for use in CI: check git commits, barf if no tests added. # 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 # Docs-only changes are excused
if [[ "${CIRRUS_CHANGE_TITLE}" =~ CI:DOCS ]]; then if [[ "${CIRRUS_CHANGE_TITLE}" =~ CI:DOCS ]]; then
exit 0 exit 0
fi 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 should be good enough, but the CIRRUS envariable allows us to test
head=${CIRRUS_CHANGE_IN_REPO:-HEAD} head=${CIRRUS_CHANGE_IN_REPO:-HEAD}
# Base of this PR. Here we absolutely rely on cirrus. # Base of this PR. Here we absolutely rely on cirrus.
@ -51,14 +50,41 @@ if [[ -z "$filtered_changes" ]]; then
exit 0 exit 0
fi fi
# One last chance: perhaps the developer included the magic '[NO NEW TESTS NEEDED]' # Nope. Only allow if the github 'no-tests-needed' label is set
# string in an amended commit. if [[ -z "$CIRRUS_PR" ]]; then
if git log --format=%B ${base}..${head} | grep -F '[NO NEW TESTS NEEDED]'; then echo "$ME: cannot query github: \$CIRRUS_PR is undefined" >&2
exit 0 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 fi
cat <<EOF cat <<EOF
$(basename $0): PR does not include changes in the 'tests' directory $ME: PR does not include changes in the 'tests' directory
Please write a regression test for what you're fixing. Even if it 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 seems trivial or obvious, try to add a test that will prevent
@ -69,8 +95,8 @@ tests, possibly just adding a small step to a similar existing test.
Every second counts in CI. Every second counts in CI.
If your commit really, truly does not need tests, you can proceed If your commit really, truly does not need tests, you can proceed
by adding '[NO NEW TESTS NEEDED]' to the body of your commit message. by asking a repo maintainer to set the '$OVERRIDE_LABEL' github label.
Please think carefully before doing so. This will only be done when there's no reasonable alternative.
EOF EOF
exit 1 exit 1

51
contrib/cirrus/pr-should-include-tests.t
View File

@ -7,6 +7,13 @@
# #
ME=$(basename $0) ME=$(basename $0)
# As of 2024-02 our test script queries github, for which we need token
if [[ -z "$GITHUB_TOKEN" ]]; then
echo "$ME: Please set \$GITHUB_TOKEN" >&2
exit 1
fi
export CIRRUS_REPO_CLONE_TOKEN="$GITHUB_TOKEN"
############################################################################### ###############################################################################
# BEGIN test cases # BEGIN test cases
# #
@ -26,19 +33,19 @@ ME=$(basename $0)
# commit history, but once we do, please add a new '0' test here. # commit history, but once we do, please add a new '0' test here.
# #
tests=" tests="
0 68c9e02df db71759b1 PR 8821: multiple commits, includes tests 0 68c9e02df db71759b1 8821 multiple commits, includes tests
0 bb82c37b7 eeb4c129b PR 8832: single commit, w/tests, merge-base test 0 bb82c37b7 eeb4c129b 8832 single commit, w/tests, merge-base test
1 1f5927699 864592c74 PR 8685, multiple commits, no tests 1 1f5927699 864592c74 8685 multiple commits, no tests
0 7592f8fbb 6bbe54f2b PR 8766, no tests, but CI:DOCS in commit message 0 7592f8fbb 6bbe54f2b 8766 no tests, but CI:DOCS in commit message
0 355e38769 bfbd915d6 PR 8884, a vendor bump 0 355e38769 bfbd915d6 8884 a vendor bump
0 ffe2b1e95 e467400eb PR 8899, only .cirrus.yml 0 ffe2b1e95 e467400eb 8899 only .cirrus.yml
0 06a6fd9f2 3cc080151 PR 8695, docs-only, without CI:DOCS 0 06a6fd9f2 3cc080151 8695 docs-only, without CI:DOCS
0 a47515008 ecedda63a PR 8816, unit tests only 0 a47515008 ecedda63a 8816 unit tests only
0 caa84cd35 e55320efd PR 8565, hack/podman-socat only 0 caa84cd35 e55320efd 8565 hack/podman-socat only
0 c342583da 12f835d12 PR 8523, version.go + podman.spec.in 0 c342583da 12f835d12 8523 version.go + podman.spec.in
0 8f75ed958 7b3ad6d89 PR 8835, only a README.md change 0 8f75ed958 7b3ad6d89 8835 only a README.md change
0 b6db60e58 f06dd45e0 PR 9420, a test rename 0 b6db60e58 f06dd45e0 9420 a test rename
0 c6a896b0c 4ea5d6971 PR 11833, includes magic string 0 c6a896b0c 4ea5d6971 11833 includes magic string
" "
# The script we're testing # The script we're testing
@ -76,10 +83,10 @@ function run_test_script() {
echo "# Actual: $output" echo "# Actual: $output"
rc=1 rc=1
else else
echo "ok $testnum $testname" echo "ok $testnum $testname - rc=$expected_rc"
fi fi
else else
echo "ok $testnum $testname" echo "ok $testnum $testname - rc=$expected_rc"
fi fi
fi fi
@ -97,15 +104,6 @@ function run_test_script() {
echo "ok $testnum $rest (override with CI:DOCS)" echo "ok $testnum $rest (override with CI:DOCS)"
fi 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 tested_override=1
fi fi
fi fi
@ -119,7 +117,7 @@ rc=0
testnum=0 testnum=0
tested_override= 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 # Skip blank lines
test -z "$expected_rc" && continue 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_IN_REPO=$commit_sha
export CIRRUS_CHANGE_TITLE=$(git log -1 --format=%s $commit_sha) export CIRRUS_CHANGE_TITLE=$(git log -1 --format=%s $commit_sha)
export CIRRUS_CHANGE_MESSAGE= export CIRRUS_CHANGE_MESSAGE=
export CIRRUS_PR=$pr
run_test_script $expected_rc "$rest" run_test_script $expected_rc "PR $pr - $rest"
done <<<"$tests" done <<<"$tests"
echo "1..$testnum" echo "1..$testnum"