From ca6bd77d8e15309c9beeaaeb04ea1db1bbe6c840 Mon Sep 17 00:00:00 2001 From: Kieran Osgood Date: Thu, 10 Sep 2026 11:26:26 +0100 Subject: [PATCH] ci(e2e): enforce required results and skip empty reports Assisted-By: devx/bfa7df54-2e93-42ac-9fa0-2277f848f09f --- e2e/BITRISE.md | 17 +++--- e2e/RUNBOOK.md | 83 ++++++++++++++-------------- e2e/bitrise.yml | 18 +++--- e2e/lib/e2e_github_reporter.rb | 7 +++ e2e/lib/ios_ci_reporter.rb | 21 ++++--- e2e/scripts/report_e2e_results | 1 + e2e/scripts/report_ios_ci_results | 7 ++- e2e/test/e2e_github_reporter_test.rb | 18 ++++++ e2e/test/ios_ci_reporter_test.rb | 31 ++++++++--- 9 files changed, 130 insertions(+), 73 deletions(-) diff --git a/e2e/BITRISE.md b/e2e/BITRISE.md index a51b416a8..64375a2e3 100644 --- a/e2e/BITRISE.md +++ b/e2e/BITRISE.md @@ -46,7 +46,7 @@ The `e2e` pipeline defines a target-based pull request trigger in `e2e/bitrise.y Shared changed-file filter groups live in `.ci/changed-file-filters.yml` and are consumed by both GitHub Actions and Bitrise E2E. Each application in `e2e/config/matrix.yml` declares `changed_files_filters` by shared group name. The run-plan producer fetches the PR file list from GitHub, applies those groups, emits only matching application rows into the BrowserStack run plan, and shares `E2E_BUILD_*` variables that gate downstream Bitrise build workflows with `run_if`. -Changes that select no applications run only the lightweight `e2e-produce-browserstack-run-plan` workflow. Its empty run plan skips app builds, BrowserStack execution, and the detailed E2E reporting workflow. Bitrise still publishes the successful `ci/bitrise/e2e/pr` pipeline result, satisfying the required check. +Changes that select no applications run only the lightweight `e2e-produce-browserstack-run-plan` workflow. Its empty run plan skips app builds, BrowserStack execution, and `e2e-report`: no `Checkout Kit E2E` Check Run is created, and no sticky PR comment is created or updated. Bitrise still publishes the successful `ci/bitrise/e2e/pr` pipeline result, satisfying the required check. For example, `platforms/react-native/README.md` is excluded by the Markdown filters, and `.github/workflows/rn-test.yml` does not select an E2E application. Both changes start the planner and finish successfully without allocating app build machines or BrowserStack devices. The runtime filters make the per-application decision after the required pipeline starts. @@ -60,20 +60,17 @@ A manually started pipeline has no pull request file list, so it selects every a Like `e2e`, the `ci-ios` target-based pull request trigger has no file filter. `ci-ios` is a merge-blocking check, and a required check that never posts leaves a pull request permanently unmergeable — so the pipeline has to start on every non-draft pull request, including a docs-only one. -Selection happens inside the pipeline instead. The Linux `ci-ios-plan` workflow reads the pull request's changed files, applies the shared filter groups in `.ci/changed-file-filters.yml` through `e2e/config/ios_ci.yml`, and publishes one `CI_IOS_*` variable per job with `share-pipeline-variable`. Each macOS workflow guards on its own variable with `run_if`. A change that needs no macOS job runs the Linux plan and the report, and nothing else. +Selection happens inside the pipeline instead. The Linux `ci-ios-plan` workflow reads the pull request's changed files, applies the shared filter groups in `.ci/changed-file-filters.yml` through `e2e/config/ios_ci.yml`, and publishes one `CI_IOS_*` variable per job with `share-pipeline-variable`. Each macOS workflow guards on its own variable with `run_if`. A change that needs no macOS job runs only the Linux plan and report workflows; the report posts nothing when planning succeeded. Both required pipelines start on every non-draft pull request and select their work at runtime. A manually started `ci-ios` pipeline selects all four macOS jobs. Choose the branch and `ci-ios` pipeline from the Bitrise **Start build** page to verify the complete iOS build and test suite. Like `e2e`, `ci-ios` has no push trigger and does not run automatically after a merge to `main`. -### The check is self-posted +### Diagnostic reporting -`ci-ios-report` runs with `should_always_run: workflow` and posts the `Checkout Kit iOS` Check Run itself, through `e2e/scripts/report_ios_ci_results`. Bitrise's own pipeline status cannot tell the two kinds of not-run apart: +Bitrise posts the required `ci/bitrise/ci-ios/pr` pipeline status. `ci-ios-report` runs with `should_always_run: workflow` and uses `e2e/scripts/report_ios_ci_results` to post the diagnostic `Checkout Kit iOS` Check Run when a job was selected or `ci-ios-plan` failed or never ran. A successful plan with no selected jobs posts no Check Run; Bitrise's successful pipeline status already satisfies the required check. -- a job the plan did not select is a **pass** — there was nothing to build -- a job the plan did select but that never finished is a **failure** - -The reporter also fails when `ci-ios-plan` itself fails, rather than reporting green off an empty selection. `e2e/test/ios_ci_reporter_test.rb` pins all three cases. +The diagnostic distinguishes jobs the plan deliberately skipped from selected jobs that failed or never ran. Selected jobs count as failures unless their stages completed successfully; a failed or never-run plan also produces a failed diagnostic, even with an empty selection. The macOS workflows fail independently, so their failures turn the required pipeline status red without relying on the reporter's exit status. `e2e/test/ios_ci_reporter_test.rb` pins these cases. ### Changing which files select which job @@ -211,11 +208,11 @@ Do not pass storefront tokens or customer data through BrowserStack Maestro envi ## GitHub reporting -For pull request builds, the `e2e-report` workflow creates commit statuses, Check Runs, and sticky PR comments using the short-lived token generated by the Bitrise GitHub App. Manual branch builds have no pull request to update, so both E2E and iOS reporting workflows skip GitHub reporting without requiring a token. +For pull request builds, Bitrise posts the required `ci/bitrise/e2e/pr` pipeline status. When the run plan is non-empty, `e2e-report` creates the diagnostic `Checkout Kit E2E` Check Run and sticky PR comment using the short-lived token generated by the Bitrise GitHub App. A failed or incomplete report exits nonzero after publishing, keeping the required pipeline status aligned with the reported E2E result. Manual branch builds have no pull request to update, so both E2E and iOS reporting workflows skip GitHub reporting without requiring a token. The Bitrise project has **Project settings > Repository > Extend GitHub App permissions to builds** enabled. Bitrise exposes the build-scoped GitHub App token as `GIT_HTTP_PASSWORD`. GitHub API scripts prefer an explicit `OVERRIDE_GITHUB_TOKEN` for local runs and otherwise use `GIT_HTTP_PASSWORD`; they intentionally ignore the shared `GITHUB_TOKEN` because it is not authenticated as the GitHub App required to create Check Runs. -Every run maintains a single sticky PR comment (create-or-update via a marker). The comment always includes an "Install with Tophat" link per SDK target and the E2E results table; failing runs add direct BrowserStack evidence links. The install links and Quick Launch entries are driven by `scripts/tophat/targets.json`; see the Tophat section in `.github/CONTRIBUTING.md`. +Each pull request build with planned E2E runs maintains a single sticky PR comment (create-or-update via a marker), including green builds. An empty run plan posts no report or comment. Reports include an "Install with Tophat" link per produced SDK target and the E2E results table when results exist; failing runs add direct BrowserStack evidence links. The install links and Quick Launch entries are driven by `scripts/tophat/targets.json`; see the Tophat section in `.github/CONTRIBUTING.md`. ## Caching diff --git a/e2e/RUNBOOK.md b/e2e/RUNBOOK.md index a9d8187c4..7c3df1b1b 100644 --- a/e2e/RUNBOOK.md +++ b/e2e/RUNBOOK.md @@ -1,21 +1,21 @@ # Checkout Kit E2E Runbook -## Rollout behaviour - -The E2E pipeline always runs and always reports; it never blocks PR merges on its -own. Merge-blocking is controlled solely by whether the single **"Checkout Kit E2E"** -GitHub Check Run is marked **required** in branch protection. Keep it non-required -until the suite is stable, then make it required — no code change is needed to gate -or un-gate. This single umbrella check stays stable across matrix changes, so -requiring it never churns as applications, OS versions, or suites are added. - -The runner never hard-fails on test or infrastructure problems: every run writes a -`result.json` and exits `0`, so the report workflow always has data to publish. The -report posts one **"Checkout Kit E2E"** Check Run and one sticky PR comment carrying the -Tophat install links and the run summary; it does not post per-suite commit statuses. The -comment is posted on every build, green or red, so the build is always installable from the -PR, and failures add a loud Failures section (alongside one red check) while staying -non-blocking. +## Merge gate behavior + +The E2E pipeline starts on every non-draft pull request so the required +`ci/bitrise/e2e/pr` status always reports. Its Linux run-plan workflow applies the +changed-file filters after the pipeline starts. When no application matches, app builds, +BrowserStack execution, and `e2e-report` are skipped. No `Checkout Kit E2E` Check Run or +sticky PR comment is published; Bitrise's successful pipeline status satisfies the +required check. + +Each BrowserStack runner writes a `result.json` and exits `0` even for test or +infrastructure failures, ensuring the report workflow has evidence to publish. When runs +are planned, the report posts one diagnostic **Checkout Kit E2E** Check Run and one sticky +PR comment carrying the Tophat install links and run summary; it does not post per-suite +commit statuses. After publishing, a failed or incomplete report exits nonzero so the +required Bitrise pipeline status also fails and blocks merging. The required gate is +Bitrise's pipeline status, not the diagnostic Check Run. Failures land in `result.json` in one of two shapes: @@ -38,10 +38,10 @@ the runner starts. The report also enforces a **completeness check**: it compares the number of `result.json` files against `E2E_BROWSERSTACK_RUN_PLAN_COUNT` (the run plan row count, shared across the pipeline). If a run never reports — for example a whole -execute workflow that failed to upload — the "Checkout Kit E2E" check is forced red -and the PR comment notes the shortfall, so a missing run can never silently -pass. When the expected count is unavailable the completeness check is skipped -rather than reporting a false failure. +execute workflow that failed to upload — both the diagnostic "Checkout Kit E2E" check +and the required pipeline status fail, and the PR comment notes the shortfall. When the +expected count is unavailable the completeness check is skipped, but if no result files +exist either, the report exits nonzero without publishing. To show *why* runs are missing, the report reads the pipeline workflow roster from `BITRISEIO_FINISHED_WORKFLOWS` and names the stages that failed or never @@ -112,35 +112,38 @@ Use the GitHub Check Run or sticky PR comment first. Failure summaries should in BrowserStack artifact links require BrowserStack App Automate access. Sign in to [BrowserStack App Automate](https://app-automate.browserstack.com/dashboard/v2/builds) before opening evidence links. The report keeps a single sticky PR comment, identified by a hidden marker, and updates it -in place on every build, so green runs never add a second comment. Because that comment -always carries the Tophat install links, it is posted even on a fully green run — a passing -build stays installable from the PR. +in place on each pull request build with planned E2E runs, including fully green runs. +Produced SDK targets have Tophat install links, so a passing build stays installable from +the PR. An empty run plan creates no Check Run and leaves any existing sticky comment +untouched. ## The iOS check failed or never posted -`Checkout Kit iOS` comes from the `ci-ios` pipeline, described in `BITRISE.md`. Three -layers can break, and the symptom tells you which one. Work down the list in order. +Bitrise posts the required `ci/bitrise/ci-ios/pr` pipeline status. `Checkout Kit iOS` is a +diagnostic Check Run from `ci-ios-report`, described in `BITRISE.md`. Check the required +pipeline status before treating an absent diagnostic as a failure. -**The check never appears.** The pipeline did not start. Its target-based pull request +**The required pipeline status never appears.** The pipeline did not start. Its target-based pull request trigger has no file filter, so the usual cause is the branch head: Bitrise reads the pipeline trigger from the pull request's own commit, and a branch older than the trigger never starts it. Rebase on `main` and push. The trigger also sets `draft_enabled: false`, so a draft posts nothing until it is marked ready. -**The check is red but every job says skipped.** `ci-ios-plan` failed, and the reporter -refuses to call an empty selection green. Open that workflow's log. It fetches the changed -file list from GitHub and reads `e2e/config/ios_ci.yml`, so the usual causes are an expired -build token or a malformed config file. - -**The check is red and names a job.** That macOS workflow failed or never finished. The -reporter lists a selected job that produced no result as a failure, so a timeout and a -compile error look different in the summary: a timeout shows as missing, a compile error -shows as failed. Both link back to the Bitrise pipeline. - -**The check is green and every job says skipped.** Expected on a change that touches no -iOS input — documentation, Android, or web. `ci-ios-plan` and `ci-ios-report` still run, -which costs about a minute on Linux. To confirm the selection is right rather than empty by -accident, run the plan locally against the same file list: +**The diagnostic check is red but every job says skipped.** `ci-ios-plan` failed or never +ran, and the reporter refuses to call an empty selection green. Inspect the pipeline and +that workflow's log, if it started. It fetches the changed file list from GitHub and reads +`e2e/config/ios_ci.yml`, so the usual causes are an expired build token or a malformed +config file. + +**The diagnostic check is red and names a job.** That macOS workflow failed or never +finished. A selected job that never ran is still a failure, rather than an intentional +skip. Follow the pipeline link to inspect the failed or missing workflow. + +**The required pipeline is green and no diagnostic check was posted.** Expected on a +change that touches no iOS input — documentation, Android, or web. `ci-ios-plan` and +`ci-ios-report` still run on Linux, but a successful plan with no selected job posts +nothing. To confirm the selection is right rather than empty by accident, run the plan +locally against the same file list: ```bash ruby e2e/scripts/ios_ci_run_plan selected-jobs --changed-file diff --git a/e2e/bitrise.yml b/e2e/bitrise.yml index 2e083b3bf..6f4a445b3 100644 --- a/e2e/bitrise.yml +++ b/e2e/bitrise.yml @@ -37,9 +37,9 @@ tools: pipelines: e2e: - # The required ci/bitrise/e2e/pr check must report for every ready PR. - # Select applications in the run plan so unrelated changes still finish - # the pipeline successfully without starting builds or BrowserStack runs. + # Deliberately unfiltered: the required ci/bitrise/e2e/pr status must report for + # every ready PR. The Linux run plan selects applications at runtime, so an + # unrelated change finishes with no app builds, no BrowserStack runs, and no report. triggers: pull_request: - source_branch: "*" @@ -81,6 +81,10 @@ pipelines: - e2e-build-kotlin-android - e2e-build-swift-ios parallel: $E2E_BROWSERSTACK_RUN_PLAN_PARALLEL_COUNT + # Gated with the execute stage: an empty run plan has nothing to publish, and the + # pipeline's own successful status is what satisfies the required check. When runs + # are planned this workflow owns the verdict and exits non-zero on a failed or + # missing result, which is what turns ci/bitrise/e2e/pr red. e2e-report: run_if: expression: '{{ enveq "E2E_HAS_E2E_RUNS" "true" }}' @@ -93,8 +97,8 @@ pipelines: ci-ios: # Deliberately unfiltered. ci-ios is a required check, and a required check that # never posts leaves a pull request permanently unmergeable. The cheap Linux - # ci-ios-plan workflow decides which macOS jobs run, and ci-ios-report always posts - # — including for a change that needs no macOS job at all. + # ci-ios-plan workflow decides which macOS jobs run, and ci-ios-report owns the + # verdict whenever a job was selected. triggers: pull_request: - source_branch: "*" @@ -122,8 +126,8 @@ pipelines: expression: '{{ enveq "CI_IOS_REACT_NATIVE_TEST_IOS" "true" }}' depends_on: - ci-ios-plan - # No run_if: this workflow is the required check, so it posts on every build, - # including one where ci-ios-plan selected nothing and every job was skipped. + # No run_if: this workflow runs on every build. It posts when a job was selected + # or ci-ios-plan failed or never ran, but not when a successful plan selected no job. ci-ios-report: depends_on: - ci-ios-swift-package-tests diff --git a/e2e/lib/e2e_github_reporter.rb b/e2e/lib/e2e_github_reporter.rb index 66b72f614..974cc3717 100644 --- a/e2e/lib/e2e_github_reporter.rb +++ b/e2e/lib/e2e_github_reporter.rb @@ -86,6 +86,13 @@ def check_run_payload } end + # e2e-report is the only stage that sees the BrowserStack verdict: a runner records a + # failure into result.json and still exits 0, so the report's exit status is what makes + # the required pipeline check fail. + def successful? + conclusion == "success" + end + private def results_table diff --git a/e2e/lib/ios_ci_reporter.rb b/e2e/lib/ios_ci_reporter.rb index 5cb83d6a0..7f9cc6bbe 100644 --- a/e2e/lib/ios_ci_reporter.rb +++ b/e2e/lib/ios_ci_reporter.rb @@ -3,12 +3,7 @@ require_relative "../../scripts/lib/json_http_client" require_relative "bitrise_pipeline_stages" -# Publishes the macOS CI pipeline outcome to GitHub as one check run. -# -# Bitrise's own commit status covers the whole pipeline, which is the wrong shape for a -# merge gate: a job the change does not need is skipped, and a skipped Bitrise workflow -# is indistinguishable from one the pipeline never reached. Only the gate's own selection -# separates the two, so the pipeline reports its own check instead. +# Publishes the selected macOS CI pipeline outcome to GitHub as a diagnostic check run. class IOSCIReporter CHECK_NAME = "Checkout Kit iOS" PLAN_STAGE_NAME = "ci-ios-plan" @@ -24,8 +19,20 @@ def initialize(job_ids:, selected_job_ids:, repository:, sha:, token:, stages:, @pipeline_url = pipeline_url end + # Returns false when there was nothing to report. A change that selects no macOS job + # has no iOS verdict, and the pipeline's own successful status already satisfies the + # required ci/bitrise/ci-ios/pr check, so a green "nothing ran" check run is noise on + # an unrelated pull request. A plan that failed or never ran still reports: an empty + # selection is then a symptom rather than a decision. def publish! + return false if nothing_to_report? + client.post_json("/repos/#{@repository}/check-runs", check_run_payload) + true + end + + def nothing_to_report? + @selected_job_ids.empty? && problem_stages.empty? end def check_run_payload @@ -51,7 +58,7 @@ def markdown_summary lines.concat(@job_ids.empty? ? [] : job_table) if @selected_job_ids.empty? lines << "" - lines << "No iOS job ran for this change." + lines << "No iOS job was selected for this change." end lines.concat(pipeline_link_lines) lines.join("\n") diff --git a/e2e/scripts/report_e2e_results b/e2e/scripts/report_e2e_results index a3c133b7d..ee1a0b39e 100755 --- a/e2e/scripts/report_e2e_results +++ b/e2e/scripts/report_e2e_results @@ -83,3 +83,4 @@ reporter = E2EGitHubReporter.new( ) reporter.publish! puts reporter.markdown_summary +exit 1 unless reporter.successful? diff --git a/e2e/scripts/report_ios_ci_results b/e2e/scripts/report_ios_ci_results index f13d16ef3..1b181b277 100755 --- a/e2e/scripts/report_ios_ci_results +++ b/e2e/scripts/report_ios_ci_results @@ -58,5 +58,8 @@ reporter = IOSCIReporter.new( stages: stages, pipeline_url: options[:pipeline_url] ) -reporter.publish! -puts reporter.markdown_summary +if reporter.publish! + puts reporter.markdown_summary +else + puts "No iOS job was selected for this change; no #{IOSCIReporter::CHECK_NAME} check run posted." +end diff --git a/e2e/test/e2e_github_reporter_test.rb b/e2e/test/e2e_github_reporter_test.rb index 6a3f5958a..edbdafd7f 100644 --- a/e2e/test/e2e_github_reporter_test.rb +++ b/e2e/test/e2e_github_reporter_test.rb @@ -72,6 +72,24 @@ def result(target) {"target" => target, "application_id" => target, "passed" => true, "execute" => "."} end + def test_complete_green_report_is_successful + report = reporter(results: [result("swift")], expected: 1) + + assert_predicate report, :successful? + end + + def test_failed_result_is_not_successful + report = reporter(results: [result("swift").merge("passed" => false)], expected: 1) + + refute_predicate report, :successful? + end + + def test_missing_result_is_not_successful + report = reporter(results: [], expected: 1) + + refute_predicate report, :successful? + end + def test_install_table_lists_every_produced_target body = reporter(results: [result("react-native"), result("swift"), result("kotlin")]).comment_body diff --git a/e2e/test/ios_ci_reporter_test.rb b/e2e/test/ios_ci_reporter_test.rb index af7ded7ff..6264136b0 100644 --- a/e2e/test/ios_ci_reporter_test.rb +++ b/e2e/test/ios_ci_reporter_test.rb @@ -30,7 +30,7 @@ def reporter(selected_job_ids:, stages:, job_ids: nil) ) end - def test_the_check_name_is_the_literal_the_ruleset_pins + def test_the_check_name_identifies_the_ios_diagnostic assert_equal "Checkout Kit iOS", IOSCIReporter::CHECK_NAME assert_equal "Checkout Kit iOS", reporter(selected_job_ids: [], stages: stage_roster).check_run_payload.fetch(:name) end @@ -72,11 +72,15 @@ def test_a_malformed_stage_roster_fails def test_an_absent_plan_stage_fails stages = stage_roster(workflow("ci-ios-swift-package-tests")) - report = reporter(selected_job_ids: ["swift-package-tests"], stages: stages) - assert_equal "failure", report.conclusion - assert_includes report.markdown_summary, "ci-ios-plan" - assert_includes report.markdown_summary, "did not run" + [[], ["swift-package-tests"]].each do |selected_job_ids| + report = reporter(selected_job_ids: selected_job_ids, stages: stages) + + assert_equal "failure", report.conclusion + refute_predicate report, :nothing_to_report? + assert_includes report.markdown_summary, "ci-ios-plan" + assert_includes report.markdown_summary, "did not run" + end end def test_an_absent_selected_job_stage_fails @@ -124,14 +128,27 @@ def test_a_failed_plan_stage_fails_even_though_no_job_was_selected report = reporter(selected_job_ids: [], stages: stages) assert_equal "failure", report.conclusion + refute_predicate report, :nothing_to_report? assert_includes report.markdown_summary, "ci-ios-plan" + assert_includes report.markdown_summary, "No iOS job was selected for this change." end - def test_no_selected_job_with_a_green_plan_passes + def test_no_selected_job_with_a_green_plan_posts_nothing report = reporter(selected_job_ids: [], stages: stage_roster(workflow("ci-ios-plan"))) assert_equal "success", report.conclusion - assert_includes report.markdown_summary, "No iOS job ran for this change" + assert_predicate report, :nothing_to_report? + refute report.publish! + end + + def test_a_selected_job_is_always_reported + stages = stage_roster( + workflow("ci-ios-plan"), + workflow("ci-ios-swift-package-tests") + ) + report = reporter(selected_job_ids: ["swift-package-tests"], stages: stages) + + refute_predicate report, :nothing_to_report? end # ci-ios-report is still running while it writes this check, so its own stage always