From fc1c695cd4c4b366e8bd99cd191378f5fba51316 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Thu, 14 May 2026 00:50:49 +0000 Subject: [PATCH] fix(ci): change status-check to failure-only OR logic for resilience The status-check consolidation job previously used != "success" AND logic which treated skipped or cancelled jobs as pipeline failures. Changed to == "failure" OR logic so only genuine errors (failure) break the pipeline, while benign states (skipped, cancelled) are accepted. Also adds BDD scenarios verifying failure-based checking and resilience coverage. --- .forgejo/workflows/ci.yml | 32 ++++++----- features/ci_workflow_validation.feature | 12 ++++ .../steps/ci_workflow_validation_steps.py | 55 +++++++++++++++++++ 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 6a6d01c57..18ec791c3 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -576,18 +576,22 @@ jobs: echo "helm: ${{ needs.helm.result }}" echo "push-validation: ${{ needs.push-validation.result }}" - if [ "${{ needs.lint.result }}" != "success" ] || \ - [ "${{ needs.typecheck.result }}" != "success" ] || \ - [ "${{ needs.security.result }}" != "success" ] || \ - [ "${{ needs.quality.result }}" != "success" ] || \ - [ "${{ needs.unit_tests.result }}" != "success" ] || \ - [ "${{ needs.integration_tests.result }}" != "success" ] || \ - [ "${{ needs.coverage.result }}" != "success" ] || \ - [ "${{ needs.build.result }}" != "success" ] || \ - [ "${{ needs.docker.result }}" != "success" ] || \ - [ "${{ needs.helm.result }}" != "success" ] || \ - [ "${{ needs.push-validation.result }}" != "success" ]; then - echo "FAILED: One or more required jobs did not succeed" - exit 1 + # Only fail on genuine failures, not skipped or cancelled jobs. + # Skipped/cancelled results are benign (e.g., when a dependency job + # was skipped due to an earlier failure) and should not cause the + # consolidation step to block the pipeline unnecessarily. + if [ "${{ needs.lint.result }}" == "failure" ] || \ + [ "${{ needs.typecheck.result }}" == "failure" ] || \ + [ "${{ needs.security.result }}" == "failure" ] || \ + [ "${{ needs.quality.result }}" == "failure" ] || \ + [ "${{ needs.unit_tests.result }}" == "failure" ] || \ + [ "${{ needs.integration_tests.result }}" == "failure" ] || \ + [ "${{ needs.coverage.result }}" == "failure" ] || \ + [ "${{ needs.build.result }}" == "failure" ] || \ + [ "${{ needs.docker.result }}" == "failure" ] || \ + [ "${{ needs.helm.result }}" == "failure" ] || \ + [ "${{ needs.push-validation.result }}" == "failure" ]; then + echo "FAILED: One or more required jobs failed" + exit 1 fi - echo "All required CI checks passed" + echo "All required CI checks passed (skipped and cancelled jobs are acceptable)" diff --git a/features/ci_workflow_validation.feature b/features/ci_workflow_validation.feature index 0e124184b..d5b2d2644 100644 --- a/features/ci_workflow_validation.feature +++ b/features/ci_workflow_validation.feature @@ -186,3 +186,15 @@ Feature: CI workflow validation Given the CI workflow file at ".forgejo/workflows/ci.yml" When I parse the CI workflow YAML Then the job "status-check" should depend on "helm" + + # --- Status-check resilience logic --- + + Scenario: Status-check uses failure-only OR check (not success-only AND check) + Given the CI workflow file at ".forgejo/workflows/ci.yml" + When I parse the CI workflow YAML + Then the job "status-check" should use failure-based result checking + + Scenario: Status-check script comments explain skipped jobs are acceptable + Given the CI workflow file at ".forgejo/workflows/ci.yml" + When I parse the CI workflow YAML + Then the status-check step should mention that skipped and cancelled jobs are acceptable diff --git a/features/steps/ci_workflow_validation_steps.py b/features/steps/ci_workflow_validation_steps.py index 39983d2cd..2321bca33 100644 --- a/features/steps/ci_workflow_validation_steps.py +++ b/features/steps/ci_workflow_validation_steps.py @@ -189,3 +189,58 @@ def step_then_at_least_one_job_uses_cache(context): raise AssertionError( "No job in the CI workflow uses actions/cache for dependency caching" ) + + +@then('the job "{job_name}" should use failure-based result checking') +def step_then_job_uses_failure_based_check(context, job_name): + """Verify the status-check job (or similar consolidation) only fails on genuine + failures using == "failure" OR logic, not != "success" AND logic that would also + flag skipped or cancelled jobs as pipeline failures.""" + jobs = context.ci_workflow.get("jobs", {}) + job = jobs.get(job_name) + if job is None: + raise AssertionError(f"Job '{job_name}' not found in workflow") + + steps = job.get("steps", []) + all_run_commands = "\n".join(step.get("run", "") for step in steps if "run" in step) + + # Must contain failure-based checks (== "failure") + has_failure_checks = '== "failure"' in all_run_commands + + # Must NOT contain success-exclusion checks (!= "success") that would misclassify + # skipped/cancelled jobs as failures. The correct logic only fails on genuine errors. + has_success_exclusions = '!= "success"' in all_run_commands + + if not has_failure_checks: + raise AssertionError( + f"Job '{job_name}' does not use failure-based result checking " + '(== "failure"). It should check for actual failures, not ' + "exclude successful results, to avoid failing on skipped/cancelled jobs." + ) + if has_success_exclusions: + raise AssertionError( + f"Job '{job_name}' still uses != \"success\" logic which classifies " + "skipped and cancelled jobs as failures. Switch to == \"failure\" OR " + "logic so only genuine errors break the pipeline." + ) + + +@then("the status-check step should mention that skipped and cancelled jobs are acceptable") +def step_then_status_check_mentions_benign_results(context): + """Verify the status-check consolidation job includes a message explaining that + skipped and cancelled job results are benign and won't cause the pipeline to fail.""" + jobs = context.ci_workflow.get("jobs", {}) + status_check = jobs.get("status-check") + if status_check is None: + raise AssertionError("Job 'status-check' not found in workflow") + + steps = status_check.get("steps", []) + all_run_commands = "\n".join(step.get("run", "") for step in steps if "run" in step) + + # The success message should indicate that skipped/cancelled are acceptable. + if "skipped and cancelled jobs are acceptable" not in all_run_commands: + raise AssertionError( + "The status-check job's success message should indicate that " + "skipped and cancelled jobs are acceptable, to clarify that only " + "genuine failures (== \"failure\") will break the pipeline." + ) -- 2.52.0