diff --git a/.github/scripts/apply-pr-quality-ruleset.sh b/.github/scripts/apply-pr-quality-ruleset.sh index 7a768236..d4436ae4 100755 --- a/.github/scripts/apply-pr-quality-ruleset.sh +++ b/.github/scripts/apply-pr-quality-ruleset.sh @@ -2,17 +2,18 @@ # apply-pr-quality-ruleset.sh — Idempotently create/update the pr-quality ruleset # # This script creates (or updates) the `pr-quality` repository ruleset that -# enforces pull-request quality gates on the default branch of +# enforces pull-request review quality gates on the default branch of # petry-projects/markets. # # Pull-request rule parameters configured (codified standard, source of truth: -# petry-projects/.github/standards/rulesets/pr-quality.json — see #323, #324): -# - required_approving_review_count: 1 -# - require_code_owner_review: true (drifted to false — issue #323) -# - required_review_thread_resolution: true -# - dismiss_stale_reviews_on_push: true (the drifted parameter — issue #324) -# - require_last_push_approval: true -# - allowed_merge_methods: ["squash"] +# petry-projects/.github/standards/rulesets/pr-quality.json — see #575/#580): +# - required_approving_review_count: 1 at least one approving review +# - require_code_owner_review: true CODEOWNERS must approve +# - required_review_thread_resolution: true all review threads resolved +# - dismiss_stale_reviews_on_push: true new pushes invalidate approvals +# - require_last_push_approval: true someone other than the last pusher +# must approve (drift fix — see #325) +# - allowed_merge_methods: ["squash"] squash-only merges # # Standard reference: # https://github.com/petry-projects/.github/blob/main/standards/github-settings.md#pr-quality--standard-ruleset-all-repositories @@ -40,8 +41,8 @@ export GH_TOKEN EXISTING_ID=$(gh api "repos/$REPO/rulesets" \ --jq ".[] | select(.name == \"$RULESET_NAME\") | .id") -PAYLOAD=$(jq -n --arg name "$RULESET_NAME" '{ - name: $name, +PAYLOAD=$(jq -n '{ + name: "pr-quality", target: "branch", enforcement: "active", conditions: { @@ -82,21 +83,7 @@ if [ -n "$EXISTING_ID" ]; then echo "Done — ruleset updated: https://github.com/$REPO/rules/$EXISTING_ID" else echo "Creating $RULESET_NAME ruleset ..." - # Capture stderr so a 422 conflict from concurrent creation doesn't terminate the script. - # On conflict, re-fetch the ID created by the concurrent run and update instead. - if RESULT=$(echo "$PAYLOAD" | gh api -X POST "repos/$REPO/rulesets" --input - 2>&1); then - NEW_ID=$(echo "$RESULT" | jq -r '.id') - echo "Done — ruleset created: https://github.com/$REPO/rules/$NEW_ID" - else - EXISTING_ID=$(gh api "repos/$REPO/rulesets" \ - --jq ".[] | select(.name == \"$RULESET_NAME\") | .id") - if [ -z "$EXISTING_ID" ]; then - echo "ERROR: POST failed and no existing $RULESET_NAME ruleset found" >&2 - echo "$RESULT" >&2 - exit 1 - fi - echo "Concurrent creation detected — updating $RULESET_NAME ruleset (id=$EXISTING_ID) ..." - echo "$PAYLOAD" | gh api -X PUT "repos/$REPO/rulesets/$EXISTING_ID" --input - > /dev/null - echo "Done — ruleset updated: https://github.com/$REPO/rules/$EXISTING_ID" - fi + RESULT=$(echo "$PAYLOAD" | gh api -X POST "repos/$REPO/rulesets" --input -) + NEW_ID=$(echo "$RESULT" | jq -r '.id') + echo "Done — ruleset created: https://github.com/$REPO/rules/$NEW_ID" fi diff --git a/.github/scripts/apply-repo-settings.sh b/.github/scripts/apply-repo-settings.sh index 5079916a..2f2d9c09 100755 --- a/.github/scripts/apply-repo-settings.sh +++ b/.github/scripts/apply-repo-settings.sh @@ -61,7 +61,7 @@ echo "Applying code-quality required-status-checks ruleset ..." "$SCRIPT_DIR/apply-code-quality-ruleset.sh" -echo "Applying pr-quality pull-request review ruleset ..." +echo "Applying pr-quality ruleset ..." "$SCRIPT_DIR/apply-pr-quality-ruleset.sh" diff --git a/.github/scripts/tests/apply-pr-quality-ruleset.bats b/.github/scripts/tests/apply-pr-quality-ruleset.bats index 79f96347..d4ff1f71 100644 --- a/.github/scripts/tests/apply-pr-quality-ruleset.bats +++ b/.github/scripts/tests/apply-pr-quality-ruleset.bats @@ -1,14 +1,22 @@ #!/usr/bin/env bats -# Tests for apply-pr-quality-ruleset.sh — static content assertions, no live API calls. +# Tests for apply-pr-quality-ruleset.sh # -# The pull_request rule parameters asserted below are the codified standard -# (petry-projects/.github/standards/rulesets/pr-quality.json), the source of -# truth referenced by issues #323 and #324. require_code_owner_review (#323) -# and dismiss_stale_reviews_on_push (#324) are the parameters that drifted to -# false and must be codified as true. +# Static checks verify the script's safety settings and target repo. +# The payload test mocks the gh CLI to capture the actual JSON sent to the +# GitHub API and uses jq to assert structure and values — avoids fragile +# grep-on-source-file assertions. SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/apply-pr-quality-ruleset.sh" +setup() { + export BATS_TMPDIR="$(mktemp -d)" + export GH_TOKEN="mock-token" +} + +teardown() { + rm -rf "$BATS_TMPDIR" +} + @test "script exists and is executable" { [ -f "$SCRIPT" ] [ -x "$SCRIPT" ] @@ -19,41 +27,46 @@ SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/apply-pr-quality-rule } @test "script targets petry-projects/markets repo" { - grep -qE '^REPO="petry-projects/markets"$' "$SCRIPT" -} - -@test "script declares the pr-quality ruleset" { - grep -qE 'RULESET_NAME\s*=\s*"pr-quality"' "$SCRIPT" -} - -@test "script requires GH_TOKEN" { - grep -q 'GH_TOKEN' "$SCRIPT" + grep -q 'petry-projects/markets' "$SCRIPT" } -@test "script sets dismiss_stale_reviews_on_push to true (issue #324)" { - grep -qE '^[[:space:]]+dismiss_stale_reviews_on_push[[:space:]]*:[[:space:]]*true[[:space:]]*,?[[:space:]]*$' "$SCRIPT" -} - -@test "script requires one approving review" { - grep -qE '^[[:space:]]+required_approving_review_count[[:space:]]*:[[:space:]]*1[[:space:]]*,?[[:space:]]*$' "$SCRIPT" -} - -@test "script requires code owner review (issue #323)" { - grep -qE '^[[:space:]]+require_code_owner_review[[:space:]]*:[[:space:]]*true[[:space:]]*,?[[:space:]]*$' "$SCRIPT" -} +@test "script generates correct ruleset payload" { + gh() { + if [ "$1" = "api" ]; then + if [ "$2" = "repos/petry-projects/markets/rulesets" ]; then + echo "" + return 0 + elif [ "$2" = "-X" ] && [ "$3" = "POST" ]; then + cat > "$BATS_TMPDIR/payload.json" + echo '{"id": 12345}' + return 0 + fi + fi + command gh "$@" + } + export -f gh -@test "script requires review thread resolution" { - grep -qE '^[[:space:]]+required_review_thread_resolution[[:space:]]*:[[:space:]]*true[[:space:]]*,?[[:space:]]*$' "$SCRIPT" -} + run bash "$SCRIPT" + [ "$status" -eq 0 ] -@test "script requires last push approval" { - grep -qE '^[[:space:]]+require_last_push_approval[[:space:]]*:[[:space:]]*true[[:space:]]*,?[[:space:]]*$' "$SCRIPT" -} + [ "$(jq -r '.name' "$BATS_TMPDIR/payload.json")" = "pr-quality" ] + [ "$(jq -r '.target' "$BATS_TMPDIR/payload.json")" = "branch" ] + [ "$(jq -r '.enforcement' "$BATS_TMPDIR/payload.json")" = "active" ] + [ "$(jq -r '.conditions.ref_name.include[0]' "$BATS_TMPDIR/payload.json")" = "~DEFAULT_BRANCH" ] + [ "$(jq -r '.rules[0].type' "$BATS_TMPDIR/payload.json")" = "pull_request" ] -@test "script restricts allowed_merge_methods to squash" { - grep -qE 'allowed_merge_methods:\s*\[\s*"squash"\s*\]' "$SCRIPT" -} + local params=".rules[0].parameters" + [ "$(jq -r "$params.required_approving_review_count" "$BATS_TMPDIR/payload.json")" -eq 1 ] + [ "$(jq -r "$params.require_code_owner_review" "$BATS_TMPDIR/payload.json")" = "true" ] + [ "$(jq -r "$params.required_review_thread_resolution" "$BATS_TMPDIR/payload.json")" = "true" ] + [ "$(jq -r "$params.dismiss_stale_reviews_on_push" "$BATS_TMPDIR/payload.json")" = "true" ] + [ "$(jq -r "$params.require_last_push_approval" "$BATS_TMPDIR/payload.json")" = "true" ] + [ "$(jq -r "$params.allowed_merge_methods[0]" "$BATS_TMPDIR/payload.json")" = "squash" ] -@test "script declares a pull_request rule type" { - grep -qE 'type:\s*"pull_request"' "$SCRIPT" + [ "$(jq '.bypass_actors | length' "$BATS_TMPDIR/payload.json")" -eq 2 ] + [ "$(jq -r '.bypass_actors[0].actor_type' "$BATS_TMPDIR/payload.json")" = "OrganizationAdmin" ] + [ "$(jq -r '.bypass_actors[0].bypass_mode' "$BATS_TMPDIR/payload.json")" = "always" ] + [ "$(jq -r '.bypass_actors[1].actor_type' "$BATS_TMPDIR/payload.json")" = "Integration" ] + [ "$(jq -r '.bypass_actors[1].bypass_mode' "$BATS_TMPDIR/payload.json")" = "always" ] + [ "$(jq '.bypass_actors[1].actor_id' "$BATS_TMPDIR/payload.json")" -eq 3167543 ] } diff --git a/.github/scripts/tests/apply-repo-settings.bats b/.github/scripts/tests/apply-repo-settings.bats index 570edf38..c5b8a9a7 100755 --- a/.github/scripts/tests/apply-repo-settings.bats +++ b/.github/scripts/tests/apply-repo-settings.bats @@ -34,6 +34,6 @@ SCRIPT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)/apply-repo-settings.s grep -q 'apply-code-quality-ruleset.sh' "$SCRIPT" } -@test "script wires in pr-quality ruleset application (issues #323, #324)" { +@test "script wires in pr-quality ruleset application (issue #325)" { grep -q 'apply-pr-quality-ruleset.sh' "$SCRIPT" } diff --git a/.github/workflows/add-to-project.yml b/.github/workflows/add-to-project.yml index 60466bba..d5b20a58 100644 --- a/.github/workflows/add-to-project.yml +++ b/.github/workflows/add-to-project.yml @@ -56,4 +56,4 @@ jobs: # `secrets: inherit` handing the reusable every org secret). secrets: INITIATIVES_APP_ID: ${{ secrets.INITIATIVES_APP_ID }} - INITIATIVES_APP_PRIVATE_KEY: ${{ secrets.INITIATIVES_APP_PRIVATE_KEY }} \ No newline at end of file + INITIATIVES_APP_PRIVATE_KEY: ${{ secrets.INITIATIVES_APP_PRIVATE_KEY }} diff --git a/.github/workflows/dev-lead.yml b/.github/workflows/dev-lead.yml index 01e794b2..fbceadb3 100644 --- a/.github/workflows/dev-lead.yml +++ b/.github/workflows/dev-lead.yml @@ -56,14 +56,14 @@ permissions: {} jobs: dev-lead: - # Pinned to the moving dev-lead/stable channel tag, not @main, so a broken + # Pinned to the moving dev-lead/v1-stable channel tag, not @main, so a broken # change to dev-lead can no longer gate its own fix (the self-host circular - # dependency). Promotion is done by moving the dev-lead/stable tag centrally; this + # dependency). Promotion is done by moving the dev-lead/v1-stable tag centrally; this # caller is never edited on release. agent_ref threads the same channel into # dev-lead's own scripts/prompts checkout. See https://github.com/petry-projects/.github/blob/main/standards/ci-standards.md#dev-lead-agent. - uses: petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@dev-lead/stable # NOSONAR(githubactions:S7637) first-party channel ref + uses: petry-projects/.github-private/.github/workflows/dev-lead-reusable.yml@dev-lead/v1-stable # NOSONAR(githubactions:S7637) first-party channel ref with: - agent_ref: dev-lead/stable + agent_ref: dev-lead/v1-stable secrets: inherit # NOSONAR(githubactions:S7635) first-party trusted reusable permissions: contents: write