From aabdd560cfe5dea17783664bf868f4053b1f0684 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Thu, 15 May 2025 18:40:19 +0200 Subject: [PATCH 01/10] Action implementation --- .../buildkite_add_trigger_step_action.rb | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb new file mode 100644 index 000000000..2ebcf3b6a --- /dev/null +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require 'open3' + +module Fastlane + module Actions + class BuildkiteAddTriggerStepAction < Action + def self.run(params) + # Extract parameters + pipeline_file = params[:pipeline_file] + build_name = File.basename(pipeline_file, '.yml') + message = params[:message] || build_name + branch = params[:branch] || `git rev-parse --abbrev-ref HEAD`.strip + environment = params[:environment] || {} + buildkite_pipeline_slug = params[:buildkite_pipeline_slug] + async = params[:async] + label = params[:label] || ":buildkite: Trigger #{build_name} on #{branch}" + + # Add the PIPELINE environment variable to the environment hash + environment = environment.merge('PIPELINE' => pipeline_file) + + # Create the trigger step YAML + trigger_yaml = { + 'steps' => [ + { + 'trigger' => buildkite_pipeline_slug, + 'label' => label, + 'async' => async, + 'build' => { + 'branch' => branch, + 'message' => message, + 'env' => environment + } + }, + ] + }.to_yaml + + # Use buildkite-agent to upload the pipeline + _stdout, stderr, _status = Open3.capture3('buildkite-agent', 'pipeline', 'upload', stdin_data: trigger_yaml) + + # Check for errors + UI.user_error!("Failed to upload pipeline: #{stderr}") unless stderr.empty? + + # Log success + UI.success("Added a trigger step to the current Buildkite build to start a new build for #{pipeline_file} on branch #{branch}") + end + + def self.description + 'Adds a trigger step to the current Buildkite pipeline to start a new build from this one' + end + + def self.details + <<~DETAILS + This action adds a `trigger` step to the current Buildkite build, to start a separate build from the current one. + + This is slightly different to `buildkite-agent pipeline upload`-ing the YAML steps of the build directly to the current build, + as this approach ensures the triggered build starts from a clean and independant context. + - This is particularly important if the build being triggered rely on the fact that the current build pushed new commits + to the Git branch and we want the new build's steps to start from the new commit. + - This is also necessary for cases where we run builds on a mirror of the original Git repo (e.g. for pipelines of repos hosted + in a private GitHub Enterprise server, mirrored on GitHub.com for CI building purposes) and we want the new build being triggered + to initiate a new sync of the Git repo as part of the CI build bootstrap process, to get the latest commits. + DETAILS + end + + def self.available_options + [ + FastlaneCore::ConfigItem.new( + key: :buildkite_pipeline_slug, + env_name: 'BUILDKITE_PIPELINE_SLUG', + description: 'The slug of the Buildkite pipeline to trigger. Defaults to the same slug as the current pipeline, so usually not necessary to provide explicitly', + type: String, + optional: false # But most likely to be auto-provided by the ENV var of the current build and thus not needed to be provided explicitly + ), + FastlaneCore::ConfigItem.new( + key: :label, + description: 'Custom label for the trigger step. If not provided, defaults to ":buildkite: Trigger {`pipeline_file`\'s basename} on {branch}"', + type: String, + optional: true + ), + FastlaneCore::ConfigItem.new( + key: :pipeline_file, + description: 'The path (relative to `.buildkite/`) to the pipeline YAML file to use for the triggered build', + type: String, + optional: false + ), + FastlaneCore::ConfigItem.new( + key: :branch, + description: 'The branch to trigger the build on. Defaults to the Git branch currently checked out at the time of running the action (which is not necessarily the same as the `BUILDKITE_BRANCH` the current build initially started on)', + type: String, + optional: true + ), + FastlaneCore::ConfigItem.new( + key: :message, + description: 'The message / title to use for the triggered build. If not provided, defaults to the `pipeline_file`\'s basename', + type: String, + optional: true + ), + FastlaneCore::ConfigItem.new( + key: :environment, + description: 'Environment variables to pass to the triggered build (in addition to the PIPELINE={pipeline_file} that will be automatically injected)', + type: Hash, + default_value: {}, + optional: true + ), + FastlaneCore::ConfigItem.new( + key: :async, + description: 'Whether to trigger the build asynchronously (true) or wait for it to complete (false). Defaults to false', + type: Boolean, + default_value: false + ), + ] + end + + def self.return_value + 'Returns true if the pipeline was successfully uploaded. Throws a `user_error!` if it failed to upload the `trigger` step to the current build' + end + + def self.authors + ['Automattic'] + end + + def self.is_supported?(platform) + true + end + + def self.example_code + [ + <<~CODE, + # Use default/inferred values for most parameters + buildkite_add_trigger_step( + pipeline_file: "release-build.yml", + environment: { "RELEASE_VERSION" => "1.2.3" }, + ) + CODE + <<~CODE, + # Use custom values for most parameters + buildkite_add_trigger_step( + label: "🚀 Trigger Release Build", + pipeline_file: "release-build.yml", + branch: "release/1.2.3", + message: "Release Build (123)", + environment: { "RELEASE_VERSION" => "1.2.3" }, + async: false, + ) + CODE + ] + end + + def self.category + :building + end + end + end +end From 2eeb61f8ef3c11e5da7dffed371b619d84e16f19 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Thu, 15 May 2025 20:00:49 +0200 Subject: [PATCH 02/10] Add unit tests --- .../buildkite_add_trigger_step_action_spec.rb | 278 ++++++++++++++++++ 1 file changed, 278 insertions(+) create mode 100644 spec/buildkite_add_trigger_step_action_spec.rb diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb new file mode 100644 index 000000000..e33cda7aa --- /dev/null +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -0,0 +1,278 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Fastlane::Actions::BuildkiteAddTriggerStepAction do + let(:pipeline_file) { 'test-pipeline.yml' } + let(:build_name) { 'test-pipeline' } + let(:branch) { 'test-branch' } + let(:message) { 'Test Build' } + let(:environment) { { 'TEST_VAR' => 'test-value' } } + let(:buildkite_pipeline_slug) { 'test-pipeline' } + let(:async) { false } + let(:label) { nil } # Will use default + + def expected_yaml( + pipeline_file: self.pipeline_file, + build_name: self.build_name, + branch: self.branch, + message: self.message, + extra_env: environment, + buildkite_pipeline_slug: self.buildkite_pipeline_slug, + async: self.async, + label: self.label + ) + # Calculate the label based on whether a custom one was provided + actual_label = label || ":buildkite: Trigger #{build_name} on #{branch}" + + # Merge the environment with PIPELINE, ensuring PIPELINE is always set correctly + actual_env = extra_env.merge('PIPELINE' => pipeline_file) + + { + 'steps' => [ + { + 'trigger' => buildkite_pipeline_slug, + 'label' => actual_label, + 'async' => async, + 'build' => { + 'branch' => branch, + 'message' => message, + 'env' => actual_env + } + }, + ] + }.to_yaml + end + + before do + # Mock the git command to return our test branch + allow(described_class).to receive(:`).with('git rev-parse --abbrev-ref HEAD').and_return(branch) + + # Mock the pipeline upload command to return a success status + allow(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: anything) + .and_return(['', '', instance_double(Process::Status, success?: true)]) + end + + context 'when all required parameters are provided' do + it 'uploads the correct pipeline YAML' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'uses the current branch when not provided' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'uses a custom label when provided' do + custom_label = '🚀 Custom Trigger Label' + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(label: custom_label)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async, + label: custom_label + ) + end + + it 'uses async: true when specified' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(async: true)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: true + ) + end + end + + context 'when the pipeline upload fails' do + it 'raises a user error with the error message' do + error_message = 'Failed to upload pipeline' + allow(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + .and_return(['', error_message, instance_double(Process::Status, success?: false)]) + + expect(FastlaneCore::UI).to receive(:user_error!).with("Failed to upload pipeline: #{error_message}") + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + end + + context 'when required parameters are missing' do + it 'raises an error when pipeline_file is not provided' do + expect do + run_described_fastlane_action( + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end.to raise_error(FastlaneCore::Interface::FastlaneError, /No value found for 'pipeline_file'/) + end + + it 'raises an error when buildkite_pipeline_slug is not provided' do + expect do + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + async: async + ) + end.to raise_error(FastlaneCore::Interface::FastlaneError, /No value found for 'buildkite_pipeline_slug'/) + end + end + + context 'when testing parameter default values and custom values' do + it 'uses the pipeline file basename as message when not provided' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(message: build_name)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'uses an empty environment hash when not provided' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(extra_env: {})) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'uses async: false when not provided' do + # This is already tested in the default case, but let's make it explicit + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug + ) + end + + it 'uses the default label when not provided' do + # This is already tested in the default case, but let's make it explicit + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'allows overriding the default message with a custom one' do + custom_message = 'Custom Build Message' + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(message: custom_message)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: custom_message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'allows overriding the default branch with a custom one' do + custom_branch = 'custom-branch' + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(branch: custom_branch)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: custom_branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'allows overriding the default environment with a custom one' do + custom_env = { 'CUSTOM_VAR' => 'custom-value' } + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(extra_env: custom_env)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: custom_env, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'preserves the PIPELINE environment variable even when custom environment is provided' do + custom_env = { 'CUSTOM_VAR' => 'custom-value', 'PIPELINE' => 'should-be-overridden.yml' } + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(extra_env: { 'CUSTOM_VAR' => 'custom-value' })) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: custom_env, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + end +end From 1545c7df0660db5c8b883ec9b2e3533727ecb818 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Thu, 15 May 2025 20:40:37 +0200 Subject: [PATCH 03/10] Fix test so it passes on CI When the tests are run from the Buildkite CI itself, the `BUILDKITE_PIPELINE_SLUG` env var would be set while the test runs, so the unit test case that is responsible for testing the case of `buildkite_pipeline_slug` parameter missing would not work as expected. So we need to ensure we temporarily unset the env var for the duration of the test being run, so that it actually runs in a context we expect it to. --- spec/buildkite_add_trigger_step_action_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index e33cda7aa..83fdbb534 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -54,6 +54,16 @@ def expected_yaml( .and_return(['', '', instance_double(Process::Status, success?: true)]) end + # Unset the `BUILDKITE_PIPELINE_SLUG` env var while running each test case + # Otherwise when we run the tests on CI, the env var would be set as part of it running as a Buildkite job, + # and this would mess up our test environment and bias the test results + around do |example| + original_value = ENV['BUILDKITE_PIPELINE_SLUG'] + ENV.delete('BUILDKITE_PIPELINE_SLUG') + example.run + ENV['BUILDKITE_PIPELINE_SLUG'] = original_value if original_value + end + context 'when all required parameters are provided' do it 'uploads the correct pipeline YAML' do expect(Open3).to receive(:capture3) From e959bd023b6021da9f05606094ba4d24c4737927 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Thu, 15 May 2025 21:23:14 +0200 Subject: [PATCH 04/10] Add support for `depends_on` parameter Defaulting to the current build's `BUILDKITE_STEP_KEY` if defined --- .../buildkite_add_trigger_step_action.rb | 14 +- .../buildkite_add_trigger_step_action_spec.rb | 136 +++++++++++++++--- 2 files changed, 130 insertions(+), 20 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb index 2ebcf3b6a..19b08b480 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -15,6 +15,7 @@ def self.run(params) buildkite_pipeline_slug = params[:buildkite_pipeline_slug] async = params[:async] label = params[:label] || ":buildkite: Trigger #{build_name} on #{branch}" + depends_on = params[:depends_on]&.then { |v| v.empty? ? nil : Array(v) } # Add the PIPELINE environment variable to the environment hash environment = environment.merge('PIPELINE' => pipeline_file) @@ -30,8 +31,9 @@ def self.run(params) 'branch' => branch, 'message' => message, 'env' => environment - } - }, + }, + 'depends_on' => depends_on + }.compact, ] }.to_yaml @@ -109,6 +111,14 @@ def self.available_options type: Boolean, default_value: false ), + FastlaneCore::ConfigItem.new( + key: :depends_on, + env_name: 'BUILDKITE_STEP_KEY', # This is the env var that Buildkite sets for the current step + description: 'The steps to depend on before triggering the build. Defaults to the current step this action is called from, if said step has a `key:` attribute set. Use an empty array to explicitly not depend on any step even if the current step has a `key`', + type: Array, + default_value: [], + optional: true + ), ] end diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index 83fdbb534..3729fad14 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -11,6 +11,7 @@ let(:buildkite_pipeline_slug) { 'test-pipeline' } let(:async) { false } let(:label) { nil } # Will use default + let(:depends_on) { nil } # Will use default def expected_yaml( pipeline_file: self.pipeline_file, @@ -20,7 +21,8 @@ def expected_yaml( extra_env: environment, buildkite_pipeline_slug: self.buildkite_pipeline_slug, async: self.async, - label: self.label + label: self.label, + depends_on: self.depends_on ) # Calculate the label based on whether a custom one was provided actual_label = label || ":buildkite: Trigger #{build_name} on #{branch}" @@ -28,19 +30,23 @@ def expected_yaml( # Merge the environment with PIPELINE, ensuring PIPELINE is always set correctly actual_env = extra_env.merge('PIPELINE' => pipeline_file) + # Build the step hash + step = { + 'trigger' => buildkite_pipeline_slug, + 'label' => actual_label, + 'async' => async, + 'build' => { + 'branch' => branch, + 'message' => message, + 'env' => actual_env + } + } + + # Add depends_on if provided + step['depends_on'] = depends_on unless depends_on.nil? + { - 'steps' => [ - { - 'trigger' => buildkite_pipeline_slug, - 'label' => actual_label, - 'async' => async, - 'build' => { - 'branch' => branch, - 'message' => message, - 'env' => actual_env - } - }, - ] + 'steps' => [step] }.to_yaml end @@ -54,14 +60,15 @@ def expected_yaml( .and_return(['', '', instance_double(Process::Status, success?: true)]) end - # Unset the `BUILDKITE_PIPELINE_SLUG` env var while running each test case - # Otherwise when we run the tests on CI, the env var would be set as part of it running as a Buildkite job, - # and this would mess up our test environment and bias the test results + # Unset both BUILDKITE_PIPELINE_SLUG and BUILDKITE_STEP_KEY env vars while running each test case around do |example| - original_value = ENV['BUILDKITE_PIPELINE_SLUG'] + original_pipeline_slug = ENV['BUILDKITE_PIPELINE_SLUG'] + original_step_key = ENV['BUILDKITE_STEP_KEY'] ENV.delete('BUILDKITE_PIPELINE_SLUG') + ENV.delete('BUILDKITE_STEP_KEY') example.run - ENV['BUILDKITE_PIPELINE_SLUG'] = original_value if original_value + ENV['BUILDKITE_PIPELINE_SLUG'] = original_pipeline_slug if original_pipeline_slug + ENV['BUILDKITE_STEP_KEY'] = original_step_key if original_step_key end context 'when all required parameters are provided' do @@ -285,4 +292,97 @@ def expected_yaml( ) end end + + context 'when testing depends_on parameter' do + it 'includes depends_on when provided with a single value' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(depends_on: ['step-1'])) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async, + depends_on: 'step-1' + ) + end + + it 'includes depends_on when provided with multiple values' do + multiple_dependencies = ['step-1', 'step-2', 'step-3'] + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(depends_on: multiple_dependencies)) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async, + depends_on: multiple_dependencies + ) + end + + it 'does not include depends_on when provided with an empty array' do + empty_dependencies = [] + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async, + depends_on: empty_dependencies + ) + end + + it 'uses BUILDKITE_STEP_KEY env var when depends_on is not provided and env var is set' do + ENV['BUILDKITE_STEP_KEY'] = 'step-from-env' + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(depends_on: ['step-from-env'])) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'does not include depends_on when not provided and BUILDKITE_STEP_KEY is not set' do + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + + it 'does not include depends_on when not provided and BUILDKITE_STEP_KEY is empty string' do + ENV['BUILDKITE_STEP_KEY'] = '' + expect(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end + end end From e14bcf0b02854765d81c37288dc85c3a69fb2d35 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Thu, 15 May 2025 21:38:16 +0200 Subject: [PATCH 05/10] Fix rubocop violation --- spec/buildkite_add_trigger_step_action_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index 3729fad14..ba89af902 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -310,7 +310,7 @@ def expected_yaml( end it 'includes depends_on when provided with multiple values' do - multiple_dependencies = ['step-1', 'step-2', 'step-3'] + multiple_dependencies = %w[step-1 step-2 step-3] expect(Open3).to receive(:capture3) .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml(depends_on: multiple_dependencies)) From 48c9bc3e815cfe08086763a1c1637ed236b934c0 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 16 May 2025 18:28:59 +0200 Subject: [PATCH 06/10] Fix pipeline upload error detection --- .../buildkite_add_trigger_step_action.rb | 4 ++-- .../buildkite_add_trigger_step_action_spec.rb | 22 +++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb index 19b08b480..cf14ee412 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -38,10 +38,10 @@ def self.run(params) }.to_yaml # Use buildkite-agent to upload the pipeline - _stdout, stderr, _status = Open3.capture3('buildkite-agent', 'pipeline', 'upload', stdin_data: trigger_yaml) + _stdout, stderr, status = Open3.capture3('buildkite-agent', 'pipeline', 'upload', stdin_data: trigger_yaml) # Check for errors - UI.user_error!("Failed to upload pipeline: #{stderr}") unless stderr.empty? + UI.user_error!("Failed to upload pipeline: #{stderr}") unless status.success? # Log success UI.success("Added a trigger step to the current Buildkite build to start a new build for #{pipeline_file} on branch #{branch}") diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index ba89af902..0976de161 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -130,8 +130,8 @@ def expected_yaml( end end - context 'when the pipeline upload fails' do - it 'raises a user error with the error message' do + context 'when pipeline upload errors' do + it 'raises a user error when the command fails' do error_message = 'Failed to upload pipeline' allow(Open3).to receive(:capture3) .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) @@ -148,6 +148,24 @@ def expected_yaml( async: async ) end + + it 'does not error when the command succeeds, even with stderr output' do + stderr_message = 'Some warning message' + allow(Open3).to receive(:capture3) + .with('buildkite-agent', 'pipeline', 'upload', stdin_data: expected_yaml) + .and_return(['', stderr_message, instance_double(Process::Status, success?: true)]) + + expect(FastlaneCore::UI).not_to receive(:user_error!) + + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end end context 'when required parameters are missing' do From 296fc86004da710f4826f0a91c965abeea451239 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Fri, 16 May 2025 19:19:17 +0200 Subject: [PATCH 07/10] Add CHANGELOG entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8fe8470..d0ef12f11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ _None_ ### New Features -_None_ +- Introduce `buildkite_add_trigger_step` action, to insert a `trigger:` step in the current pipeline to trigger a child build from the current build. [#648] ### Bug Fixes From eb52f77f16945bb2b8c9d2fe1b3ad0f7c8a65697 Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 20 May 2025 13:00:17 +0200 Subject: [PATCH 08/10] Catch early if we're running the action outside a Buildkite job See https://github.com/wordpress-mobile/release-toolkit/pull/648\#discussion_r2096368024 --- .../buildkite_add_trigger_step_action.rb | 6 ++++ .../buildkite_add_trigger_step_action_spec.rb | 28 ++++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb index cf14ee412..2094dd645 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -5,7 +5,13 @@ module Fastlane module Actions class BuildkiteAddTriggerStepAction < Action + BUILDKITE_ENV_ERROR_MESSAGE = 'This action can only be run from within a Buildkite build' + def self.run(params) + unless ENV.key?('BUILDKITE_JOB_ID') + UI.user_error!(BUILDKITE_ENV_ERROR_MESSAGE) + end + # Extract parameters pipeline_file = params[:pipeline_file] build_name = File.basename(pipeline_file, '.yml') diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index 0976de161..8d22187b6 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -60,15 +60,27 @@ def expected_yaml( .and_return(['', '', instance_double(Process::Status, success?: true)]) end - # Unset both BUILDKITE_PIPELINE_SLUG and BUILDKITE_STEP_KEY env vars while running each test case + # Stub BUILDKITE_* env vars while running each test case around do |example| original_pipeline_slug = ENV['BUILDKITE_PIPELINE_SLUG'] original_step_key = ENV['BUILDKITE_STEP_KEY'] + original_job_id = ENV['BUILDKITE_JOB_ID'] + + # Unset BUILDKITE_PIPELINE_SLUG and BUILDKITE_STEP_KEY env vars because they'd otherwise be used as default values for ConfigItems of the action ENV.delete('BUILDKITE_PIPELINE_SLUG') ENV.delete('BUILDKITE_STEP_KEY') + # Set BUILDKITE_JOB_ID to a non-empty value to satisfy the check in the action, unless `remove_job_id: true` is used in the example metadata + if example.metadata[:remove_job_id] + ENV.delete('BUILDKITE_JOB_ID') + else + ENV['BUILDKITE_JOB_ID'] = '1337' + end + example.run + ENV['BUILDKITE_PIPELINE_SLUG'] = original_pipeline_slug if original_pipeline_slug ENV['BUILDKITE_STEP_KEY'] = original_step_key if original_step_key + ENV['BUILDKITE_JOB_ID'] = original_job_id if original_job_id end context 'when all required parameters are provided' do @@ -192,6 +204,20 @@ def expected_yaml( ) end.to raise_error(FastlaneCore::Interface::FastlaneError, /No value found for 'buildkite_pipeline_slug'/) end + + it 'raises an error when BUILDKITE_JOB_ID is not set', :remove_job_id do + # NOTE: the `:remove_job_id` metadata set on this spec example is used in the `around` block to unset BUILDKITE_JOB_ID for this test + expect do + run_described_fastlane_action( + pipeline_file: pipeline_file, + branch: branch, + message: message, + environment: environment, + buildkite_pipeline_slug: buildkite_pipeline_slug, + async: async + ) + end.to raise_error(FastlaneCore::Interface::FastlaneError, Fastlane::Actions::BuildkiteAddTriggerStepAction::BUILDKITE_ENV_ERROR_MESSAGE) + end end context 'when testing parameter default values and custom values' do From b4211571cd3fbfc695294a3d0495c523c12c6efe Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 20 May 2025 13:05:30 +0200 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: Ian G. Maia --- .../actions/common/buildkite_add_trigger_step_action.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb index 2094dd645..d89aa0ed0 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -61,8 +61,8 @@ def self.details <<~DETAILS This action adds a `trigger` step to the current Buildkite build, to start a separate build from the current one. - This is slightly different to `buildkite-agent pipeline upload`-ing the YAML steps of the build directly to the current build, - as this approach ensures the triggered build starts from a clean and independant context. + This is slightly different from `buildkite-agent pipeline upload`-ing the YAML steps of the build directly to the current build, + as this approach ensures the triggered build starts from a clean and independent context. - This is particularly important if the build being triggered rely on the fact that the current build pushed new commits to the Git branch and we want the new build's steps to start from the new commit. - This is also necessary for cases where we run builds on a mirror of the original Git repo (e.g. for pipelines of repos hosted From 356855375f2ae3f5c1e7ca51107b0af28b3a270e Mon Sep 17 00:00:00 2001 From: Olivier Halligon Date: Tue, 20 May 2025 13:11:27 +0200 Subject: [PATCH 10/10] Use `sh` instead of backticks to get current branch --- .../actions/common/buildkite_add_trigger_step_action.rb | 2 +- spec/buildkite_add_trigger_step_action_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb index d89aa0ed0..70c175bb6 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_add_trigger_step_action.rb @@ -16,7 +16,7 @@ def self.run(params) pipeline_file = params[:pipeline_file] build_name = File.basename(pipeline_file, '.yml') message = params[:message] || build_name - branch = params[:branch] || `git rev-parse --abbrev-ref HEAD`.strip + branch = params[:branch] || sh('git', 'rev-parse', '--abbrev-ref', 'HEAD').strip environment = params[:environment] || {} buildkite_pipeline_slug = params[:buildkite_pipeline_slug] async = params[:async] diff --git a/spec/buildkite_add_trigger_step_action_spec.rb b/spec/buildkite_add_trigger_step_action_spec.rb index 8d22187b6..421f6fba1 100644 --- a/spec/buildkite_add_trigger_step_action_spec.rb +++ b/spec/buildkite_add_trigger_step_action_spec.rb @@ -52,7 +52,7 @@ def expected_yaml( before do # Mock the git command to return our test branch - allow(described_class).to receive(:`).with('git rev-parse --abbrev-ref HEAD').and_return(branch) + allow(described_class).to receive(:sh).with('git', 'rev-parse', '--abbrev-ref', 'HEAD').and_return("#{branch}\n") # Mock the pipeline upload command to return a success status allow(Open3).to receive(:capture3)