-
Notifications
You must be signed in to change notification settings - Fork 0
feat: implement issue #580 — Retire/delineate the detection-based apply-rulesets.sh — it diverges from the codified ruleset (#575 debt) #583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # Quality gates for the apply-rulesets script and its unit tests. | ||
| # | ||
| # Triggered on any PR that touches: | ||
| # - scripts/apply-rulesets.sh | ||
| # - test/scripts/apply-rulesets/** | ||
| # | ||
| # Gates (all must pass before merge): | ||
| # 1. shellcheck — static analysis for the applier script | ||
| # 2. bats — unit tests for detect_required_checks (guards the #580 | ||
| # Dev-Lead Agent divergence removal against regression) | ||
|
|
||
| name: Apply Rulesets Tests | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - 'scripts/apply-rulesets.sh' | ||
| - 'test/scripts/apply-rulesets/**' | ||
| - '.github/workflows/apply-rulesets-tests.yml' | ||
| push: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - 'scripts/apply-rulesets.sh' | ||
| - 'test/scripts/apply-rulesets/**' | ||
| - '.github/workflows/apply-rulesets-tests.yml' | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| concurrency: | ||
| group: apply-rulesets-tests-${{ github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Lint and bats | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| fetch-depth: 1 | ||
|
|
||
| - name: Install bats, shellcheck, and jq | ||
| run: | | ||
| set -euo pipefail | ||
| sudo apt-get update -qq | ||
| sudo apt-get install -y --no-install-recommends bats shellcheck jq | ||
|
|
||
| - name: shellcheck | ||
| run: | | ||
| set -euo pipefail | ||
| shellcheck --severity=warning -x scripts/apply-rulesets.sh | ||
|
|
||
| - name: Run bats suite | ||
| run: bats --print-output-on-failure test/scripts/apply-rulesets/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| #!/usr/bin/env bats | ||
| # Tests for detect_required_checks() in scripts/apply-rulesets.sh — specifically | ||
| # the #580 debt: the detection-based builder must NOT inject a non-codified | ||
| # `Dev-Lead Agent / dev-lead` required status check. | ||
| # | ||
| # Per #579, the Dev-Lead Agent is a per-PR review, not a merge gate — requiring | ||
| # it deadlocks workflow-touching PRs. The codified source of truth | ||
| # (standards/rulesets/code-quality.json) deliberately omits it, so the | ||
| # detection-based copy must agree and not add it back. | ||
| # | ||
| # The suite sources the script (guarded main block) and stubs `gh` so the pure | ||
| # workflow-detection logic can run without live API calls. It asserts that even | ||
| # when dev-lead.yml is present in a repo, the Dev-Lead context is not emitted, | ||
| # while the genuinely-codified centralized checks still are (no over-removal). | ||
|
|
||
| bats_require_minimum_version 1.5.0 | ||
|
|
||
| setup() { | ||
| SCRIPT="${BATS_TEST_DIRNAME}/../../../scripts/apply-rulesets.sh" | ||
| [ -f "$SCRIPT" ] || { echo "script not found: $SCRIPT" >&2; return 1; } | ||
|
|
||
| # Sourcing the script must not run main / usage (requires the BASH_SOURCE | ||
| # guard added for #580). GH_TOKEN is unset in the test env; the guard keeps | ||
| # the top-level token check from aborting the source. | ||
| # shellcheck source=/dev/null | ||
| source "$SCRIPT" | ||
|
|
||
| # Initialize mock variables to prevent unbound variable errors under set -u | ||
| MOCK_WORKFLOWS=() | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # gh stub — answers the three API shapes detect_required_checks() issues: | ||
| # 1. contents/.github/workflows → newline list of workflow names | ||
| # 2. contents/.github/workflows/<file> → base64 of the file's `.content` | ||
| # 3. code-scanning/default-setup → the default-setup state string | ||
| # | ||
| # MOCK_WORKFLOWS (array) and MOCK_CODEQL_STATE (string) parameterize each test. | ||
| # --------------------------------------------------------------------------- | ||
| gh() { | ||
| local path="${2:-}" | ||
| case "$path" in | ||
| */contents/.github/workflows) | ||
| [ "${#MOCK_WORKFLOWS[@]}" -gt 0 ] && printf '%s\n' "${MOCK_WORKFLOWS[@]}" || true | ||
| ;; | ||
| */contents/.github/workflows/sonarcloud.yml) | ||
| # Matches the org-standard standards/workflows/sonarcloud.yml (name: SonarCloud | ||
| # Analysis, job: SonarCloud). SonarCloud registers its check context directly | ||
| # as "SonarCloud" — detection must emit "SonarCloud", never the compound | ||
| # "SonarCloud Analysis / SonarCloud" that a naive workflow_name() derivation | ||
| # would produce. | ||
| printf 'name: SonarCloud Analysis\non: push\njobs:\n sonarcloud:\n name: SonarCloud\n' | base64 | ||
| ;; | ||
|
Comment on lines
+46
to
+53
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in two places: (1) Updated the sonarcloud.yml stub (bats line ~46) to include
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed implemented and verified. The sonarcloud.yml stub in |
||
| */contents/.github/workflows/ci.yml) | ||
| printf 'name: CI\njobs:\n build:\n name: build\n' | base64 | ||
| ;; | ||
| */code-scanning/default-setup) | ||
| printf '%s\n' "${MOCK_CODEQL_STATE:-configured}" | ||
| ;; | ||
| *) | ||
| return 1 | ||
| ;; | ||
| esac | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # The divergence guard: dev-lead.yml present must NOT add the Dev-Lead context. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| @test "dev-lead.yml present does NOT inject 'Dev-Lead Agent / dev-lead'" { | ||
| MOCK_WORKFLOWS=("agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") | ||
| MOCK_CODEQL_STATE="configured" | ||
| run detect_required_checks "somerepo" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" != *"Dev-Lead Agent / dev-lead"* ]] | ||
| } | ||
|
|
||
| @test "dev-lead.yml as the only workflow yields no Dev-Lead context" { | ||
| MOCK_WORKFLOWS=("dev-lead.yml") | ||
| MOCK_CODEQL_STATE="configured" | ||
| run detect_required_checks "somerepo" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" != *"Dev-Lead"* ]] | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # SonarCloud context: always the bare "SonarCloud", never the compound form. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| @test "sonarcloud.yml with 'name: SonarCloud Analysis' emits bare 'SonarCloud' context" { | ||
| # The org-standard sonarcloud.yml has name: SonarCloud Analysis. SonarCloud | ||
| # registers its check directly via the SonarCloud API as "SonarCloud", so | ||
| # the required-check context must never be "SonarCloud Analysis / SonarCloud". | ||
| MOCK_WORKFLOWS=("sonarcloud.yml") | ||
| MOCK_CODEQL_STATE="not-configured" | ||
| run detect_required_checks "somerepo" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"SonarCloud"* ]] | ||
| [[ "$output" != *"SonarCloud Analysis / SonarCloud"* ]] | ||
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # No over-removal: the genuinely-codified centralized checks still appear. | ||
| # --------------------------------------------------------------------------- | ||
|
|
||
| @test "codified centralized checks are still emitted" { | ||
| MOCK_WORKFLOWS=("agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") | ||
| MOCK_CODEQL_STATE="configured" | ||
| run detect_required_checks "somerepo" | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"agent-shield / AgentShield"* ]] | ||
| [[ "$output" == *"dependency-audit / Detect ecosystems"* ]] | ||
| [[ "$output" == *"CodeQL"* ]] | ||
| } | ||
|
|
||
| @test "detected set matches the codified code-quality contexts for these workflows" { | ||
| # With sonarcloud + agent-shield + dependency-audit + configured CodeQL, the | ||
| # detection output must equal the codified standards/rulesets/code-quality.json | ||
| # context set (order-independent) — no extra Dev-Lead injection. | ||
| MOCK_WORKFLOWS=("sonarcloud.yml" "agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") | ||
| MOCK_CODEQL_STATE="configured" | ||
| run detect_required_checks "somerepo" | ||
|
Comment on lines
+116
to
+122
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reconciled all three sources: fixed
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Confirmed reconciled and verified. |
||
| [ "$status" -eq 0 ] | ||
|
|
||
| detected=$(printf '%s\n' "$output" | sort) | ||
| codified=$(jq -r '.rules[] | select(.type=="required_status_checks") | ||
| | .parameters?.required_status_checks?[]?.context?' \ | ||
| "${BATS_TEST_DIRNAME}/../../../standards/rulesets/code-quality.json" | sort) | ||
|
don-petry marked this conversation as resolved.
|
||
| [ "$detected" = "$codified" ] | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.