buildkite_pipeline_upload: make sure environment keys and values are Strings

CHANGELOG.md

@@ -18,7 +18,8 @@ _None_
 
 ### Internal Changes
 
-_None_
+- `buildkite_pipeline_upload`: makes sure all values passed in the environment parameter are strings [#608]
+- `buildkite_pipeline_upload`: prepend `.buildkite/` to the `pipeline_file` parameter to enforce our conventions [#608]
 
 ## 12.2.1
 

lib/fastlane/plugin/wpmreleasetoolkit/actions/common/buildkite_pipeline_upload_action.rb

@@ -1,17 +1,20 @@
 module Fastlane
   module Actions
     class BuildkitePipelineUploadAction < Action
-      DEFAULT_ENV_FILE = File.join('.buildkite', 'shared-pipeline-vars').freeze
+      DEFAULT_BUILDKITE_PIPELINE_FOLDER = '.buildkite'.freeze
+      DEFAULT_ENV_FILE = File.join(DEFAULT_BUILDKITE_PIPELINE_FOLDER, 'shared-pipeline-vars').freeze
 
       def self.run(params)
         pipeline_file = params[:pipeline_file]
+        pipeline_file = File.join(DEFAULT_BUILDKITE_PIPELINE_FOLDER, pipeline_file) unless File.absolute_path?(pipeline_file)
         env_file = params[:env_file]
-        environment = params[:environment]
+        # Both keys and values need to be passed as strings otherwise Fastlane.sh will fail to parse the command.
+        environment = params[:environment].to_h { |k, v| [k.to_s, v.to_s] }
 
         UI.user_error!("Pipeline file not found: #{pipeline_file}") unless File.exist?(pipeline_file)
         UI.user_error!('This action can only be called from a Buildkite CI build') unless ENV['BUILDKITE'] == 'true'
 
-        UI.message "Adding steps from `#{pipeline_file}` to the current build"
+        UI.message("Adding steps from `#{pipeline_file}` to the current build")
 
         if env_file && File.exist?(env_file)
           UI.message(" - Sourcing environment file beforehand: #{env_file}")
@@ -31,7 +34,7 @@ def self.available_options
         [
           FastlaneCore::ConfigItem.new(
             key: :pipeline_file,
-            description: 'The path to the YAML pipeline file to upload',
+            description: 'The path to the YAML pipeline file to upload. If a relative path is provided, it will be prefixed with the `.buildkite/` folder path. Absolute paths are used as-is',
             optional: false,
             type: String
           ),

spec/buildkite_pipeline_upload_action_spec.rb

@@ -2,6 +2,7 @@
 
 describe Fastlane::Actions::BuildkitePipelineUploadAction do
   let(:pipeline_file) { 'path/to/pipeline.yml' }
+  let(:loaded_pipeline_file) { File.join('.buildkite', pipeline_file) }
   let(:env_file) { 'path/to/env_file' }
   let(:env_file_default) { Fastlane::Actions::BuildkitePipelineUploadAction::DEFAULT_ENV_FILE }
   let(:environment) { { 'AKEY' => 'AVALUE' } }
@@ -23,14 +24,14 @@
     end
 
     it 'raises an error when pipeline_file does not exist' do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(false)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(false)
       expect do
         run_described_fastlane_action(pipeline_file: pipeline_file)
       end.to raise_error(FastlaneCore::Interface::FastlaneError, /Pipeline file not found/)
     end
 
     it 'raises an error when not running on Buildkite' do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(true)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true)
       allow(ENV).to receive(:[]).with('BUILDKITE').and_return(nil)
 
       expect do
@@ -39,10 +40,10 @@
     end
 
     it 'passes the environment hash to the shell command' do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(true)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true)
       expect(Fastlane::Action).to receive(:sh).with(
         environment,
-        'buildkite-agent', 'pipeline', 'upload', pipeline_file
+        'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file
       )
       expect_upload_pipeline_message
 
@@ -53,11 +54,11 @@
     end
 
     it 'passes the environment hash to the shell command also with an env_file' do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(true)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true)
       allow(File).to receive(:exist?).with(env_file).and_return(true)
       expect(Fastlane::Action).to receive(:sh).with(
         environment,
-        "source #{env_file.shellescape} && buildkite-agent pipeline upload #{pipeline_file.shellescape}"
+        "source #{env_file.shellescape} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}"
       )
       expect_upload_pipeline_message
       expect_sourcing_env_file_message(env_file)
@@ -72,13 +73,13 @@
 
   describe 'pipeline upload' do
     before do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(true)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true)
     end
 
     it 'calls the right command to upload the pipeline without env_file' do
       expect(Fastlane::Action).to receive(:sh).with(
         environment_default,
-        'buildkite-agent', 'pipeline', 'upload', pipeline_file
+        'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file
       )
       expect_upload_pipeline_message
 
@@ -89,7 +90,7 @@
       allow(File).to receive(:exist?).with(env_file).and_return(true)
       expect(Fastlane::Action).to receive(:sh).with(
         environment_default,
-        "source #{env_file.shellescape} && buildkite-agent pipeline upload #{pipeline_file.shellescape}"
+        "source #{env_file.shellescape} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}"
       )
       expect_upload_pipeline_message
       expect_sourcing_env_file_message(env_file)
@@ -105,7 +106,7 @@
       allow(File).to receive(:exist?).with(non_existent_env_file).and_return(false)
       expect(Fastlane::Action).to receive(:sh).with(
         environment_default,
-        'buildkite-agent', 'pipeline', 'upload', pipeline_file
+        'buildkite-agent', 'pipeline', 'upload', loaded_pipeline_file
       )
       expect(Fastlane::UI).not_to receive(:message).with(/Sourcing environment file/)
 
@@ -119,7 +120,7 @@
       allow(File).to receive(:exist?).with(env_file_default).and_return(true)
       expect(Fastlane::Action).to receive(:sh).with(
         environment_default,
-        "source #{env_file_default} && buildkite-agent pipeline upload #{pipeline_file.shellescape}"
+        "source #{env_file_default} && buildkite-agent pipeline upload #{loaded_pipeline_file.shellescape}"
       )
       expect_upload_pipeline_message
       expect_sourcing_env_file_message(env_file_default)
@@ -128,11 +129,48 @@
         pipeline_file: pipeline_file
       )
     end
+
+    it 'prefixes relative pipeline paths with the `.buildkite/` folder' do
+      relative_path = 'relative/pipeline.yml'
+      expected_path = File.join('.buildkite', relative_path)
+      allow(File).to receive(:exist?).with(expected_path).and_return(true)
+      allow(File).to receive(:absolute_path?).with(relative_path).and_return(false)
+
+      expect(Fastlane::Action).to receive(:sh).with(
+        environment_default,
+        'buildkite-agent', 'pipeline', 'upload', expected_path
+      )
+      expect(Fastlane::UI).to receive(:message).with(
+        "Adding steps from `#{expected_path}` to the current build"
+      )
+
+      run_described_fastlane_action(
+        pipeline_file: relative_path
+      )
+    end
+
+    it 'uses absolute pipeline paths as-is' do
+      absolute_path = '/absolute/path/to/pipeline.yml'
+      allow(File).to receive(:exist?).with(absolute_path).and_return(true)
+      allow(File).to receive(:absolute_path?).with(absolute_path).and_return(true)
+
+      expect(Fastlane::Action).to receive(:sh).with(
+        environment_default,
+        'buildkite-agent', 'pipeline', 'upload', absolute_path
+      )
+      expect(Fastlane::UI).to receive(:message).with(
+        "Adding steps from `#{absolute_path}` to the current build"
+      )
+
+      run_described_fastlane_action(
+        pipeline_file: absolute_path
+      )
+    end
   end
 
   describe 'error handling' do
     it 'raises an error when the pipeline upload fails' do
-      allow(File).to receive(:exist?).with(pipeline_file).and_return(true)
+      allow(File).to receive(:exist?).with(loaded_pipeline_file).and_return(true)
       allow(Fastlane::Action).to receive(:sh).and_raise(StandardError.new('Upload failed'))
 
       expect do
@@ -145,7 +183,7 @@
 
   def expect_upload_pipeline_message
     expect(Fastlane::UI).to receive(:message).with(
-      "Adding steps from `#{pipeline_file}` to the current build"
+      "Adding steps from `#{loaded_pipeline_file}` to the current build"
     )
   end