diff --git a/.bundle/config b/.bundle/config new file mode 100644 index 0000000..2369228 --- /dev/null +++ b/.bundle/config @@ -0,0 +1,2 @@ +--- +BUNDLE_PATH: "vendor/bundle" diff --git a/.gitignore b/.gitignore index aa7e664..1d23dfb 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,6 @@ # Coverage directory used by simplycov coverage + +# Bundler +vendor/bundle diff --git a/README.md b/README.md index f65c066..0990c9a 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ “Dobby is free.” A Github action which provides chat-ops functionality. You can comment on a pull request to perform various operations. -Currently it supports bumping version files in Ruby, Python and Javascript. The version file (see below) must specify the version +Currently it supports bumping version files in Ruby, Python and Javascript. When bumping versions, Dobby first merges the default branch into your PR branch to ensure it's up to date, then proceeds with the version bump. The version file (see below) must specify the version as a key-value pair separated by either ":" or "=", e.g. `VERSION: '1.2.3'` or `version = 1.2.3` ## Bump version @@ -78,6 +78,11 @@ jobs: ``` where semver level can be minor/major/patch. -2. You can see bot will add a comment on Pull request. +2. When you run the command, Dobby will: + - First merge the default branch (e.g., main/master) into your PR branch to ensure it's up to date + - Then bump the version in the specified files if the merge is successful + - If the merge fails (e.g., due to conflicts), the version bump will not proceed and you'll need to resolve conflicts manually + +3. You can see bot will add a comment on Pull request. ![Version update comment](docs/images/version-update.png) diff --git a/lib/utils/bump.rb b/lib/utils/bump.rb index e0dc13f..4bf607d 100644 --- a/lib/utils/bump.rb +++ b/lib/utils/bump.rb @@ -2,6 +2,7 @@ require_relative 'content' require_relative 'commit' +require_relative 'merge' SEMVER = / ["']? # Optional quotes @@ -52,11 +53,38 @@ def initialize(config, level) @other_version_file_paths = config.other_version_file_paths @other_version_patterns = config.other_version_patterns @repo = payload['repository']['full_name'] + @merge_message = '' # Initialize to avoid nil pointer exception assign_pr_attributes!(payload['issue']['number']) calculate_bumping_data! end def bump_everything + merge_result = perform_merge + return merge_result unless merge_result.nil? + + bump_versions + end + + private + + def perform_merge + merge = Merge.new(@config) + merge_result = merge.merge_base_into_head + + unless merge_result[:success] + return "### :boom: Merge failed :boom:\n\n#{merge_result[:message]}" + end + + @merge_message = "### Merge complete\n\n#{merge_result[:message]}\n\n" + + # Recalculate bumping data after merge since the merge may have introduced + # changes to version files from the base branch. This ensures we're working + # with the most up-to-date content after the merge is complete. + calculate_bumping_data! + nil # Return nil to indicate merge succeeded and we should proceed + end + + def bump_versions commit = Commit.new(@config) files = [] files_messages = {} @@ -68,11 +96,9 @@ def bump_everything end commit.multiple_files(files, "Bump #{@level} version") if files.any? - generate_message(files_messages) + @merge_message + generate_message(files_messages) end - private - def calculate_bumping_data! base_branch_content = Content.new(config: @config, ref: @base_branch, path: @version_file_path) @version = fetch_version(base_branch_content) diff --git a/lib/utils/merge.rb b/lib/utils/merge.rb new file mode 100644 index 0000000..4f106c6 --- /dev/null +++ b/lib/utils/merge.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# Merge base branch into head branch of a PR +class Merge + def initialize(config) + @config = config + @client = config.client + payload = config.payload + @repo = payload['repository']['full_name'] + pr_number = payload['issue']['number'] + pull_req = @client.pull_request(@repo, pr_number) + @head_branch = pull_req['head']['ref'] + @base_branch = pull_req['base']['ref'] + end + + def merge_base_into_head + puts "::notice title=Merging::Merging #{@base_branch} into #{@head_branch}" + begin + result = @client.merge( + @repo, @head_branch, @base_branch, + commit_message: "Merge #{@base_branch} into #{@head_branch}" + ) + if result + puts "::notice title=Merge successful::Successfully merged #{@base_branch} into #{@head_branch}" + { success: true, message: "Successfully merged #{@base_branch} into #{@head_branch}" } + else + puts "::notice title=Already up to date::Branch #{@head_branch} is already up to date with #{@base_branch}" + { success: true, message: "Branch #{@head_branch} is already up to date with #{@base_branch}" } + end + rescue Octokit::Conflict => e + puts "::error title=Merge conflict::Failed to merge #{@base_branch} into #{@head_branch}: #{e.message}" + { success: false, message: "Failed to merge #{@base_branch} into #{@head_branch}: merge conflict detected" } + rescue StandardError => e + puts "::error title=Merge failed::Failed to merge #{@base_branch} into #{@head_branch}: #{e.message}" + { success: false, message: "Failed to merge #{@base_branch} into #{@head_branch}: #{e.message}" } + end + end +end diff --git a/spec/lib/action_spec.rb b/spec/lib/action_spec.rb index cfdba2a..6fa751a 100644 --- a/spec/lib/action_spec.rb +++ b/spec/lib/action_spec.rb @@ -35,6 +35,13 @@ 'head' => { 'ref' => 'my_branch' }, 'base' => { 'ref' => 'master' } }) + # Mock the merge call for all tests + allow(client).to receive(:merge).with( + repo_full_name, + 'my_branch', + 'master', + commit_message: 'Merge master into my_branch' + ).and_return({ 'sha' => 'merge-sha' }) end describe '#initiate_version_update' do diff --git a/spec/lib/utils/bump_spec.rb b/spec/lib/utils/bump_spec.rb index f7a9e78..c7533ac 100644 --- a/spec/lib/utils/bump_spec.rb +++ b/spec/lib/utils/bump_spec.rb @@ -38,6 +38,14 @@ 'base' => { 'ref' => 'base_branch' } }) + # Mock the merge call + allow(client).to receive(:merge).with( + 'owner/repo', + 'head_branch', + 'base_branch', + commit_message: 'Merge base_branch into head_branch' + ).and_return({ 'sha' => 'merge-sha' }) + allow(Commit).to receive(:new).with(config).and_return(commit) allow(commit).to receive(:multiple_files) end @@ -81,14 +89,9 @@ bump = Bump.new(config, 'patch') expect(commit).not_to receive(:multiple_files) - expect do - bump.bump_everything - end.to output( - "::notice title=Nothing to update::The desired version bump is already present for: " \ - "#{other_version_file_paths[0]}\n" \ - "::notice title=Nothing to update::The desired version bump is already present for: " \ - "#{version_file_path}\n" - ).to_stdout + output = bump.bump_everything + expect(output).to include('Merge complete') + expect(output).to include('Nothing to update') end it 'retains modified version file content' do @@ -126,5 +129,25 @@ ) bump.bump_everything end + + it 'returns error message when merge fails' do + allow(head_content).to receive(:content).and_return('version: 1.0.0') + allow(base_content).to receive(:content).and_return('version: 1.0.0') + + # Override the default merge mock to simulate a failure + allow(client).to receive(:merge).with( + 'owner/repo', + 'head_branch', + 'base_branch', + commit_message: 'Merge base_branch into head_branch' + ).and_raise(Octokit::Conflict.new) + + bump = Bump.new(config, 'patch') + expect(commit).not_to receive(:multiple_files) + result = bump.bump_everything + + expect(result).to include('Merge failed') + expect(result).to include('merge conflict detected') + end end end diff --git a/spec/lib/utils/merge_spec.rb b/spec/lib/utils/merge_spec.rb new file mode 100644 index 0000000..2ba3c1f --- /dev/null +++ b/spec/lib/utils/merge_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' +require 'utils/merge' +require 'config' +require 'octokit' + +RSpec.describe Merge do + let(:config) { instance_double(Config) } + let(:client) { instance_double(Octokit::Client) } + let(:payload) do + { + 'repository' => { 'full_name' => 'owner/repo' }, + 'issue' => { 'number' => 123 } + } + end + + before do + allow(config).to receive_messages(client: client, payload: payload) + allow(client).to receive(:pull_request).with('owner/repo', 123).and_return( + { + 'head' => { 'ref' => 'feature-branch' }, + 'base' => { 'ref' => 'main' } + } + ) + end + + describe '#merge_base_into_head' do + it 'successfully merges base branch into head branch' do + expect(client).to receive(:merge).with( + 'owner/repo', + 'feature-branch', + 'main', + commit_message: 'Merge main into feature-branch' + ).and_return({ 'sha' => 'abc123' }) + + merge = Merge.new(config) + result = merge.merge_base_into_head + + expect(result[:success]).to be true + expect(result[:message]).to include('Successfully merged main into feature-branch') + end + + it 'handles when branch is already up to date' do + expect(client).to receive(:merge).with( + 'owner/repo', + 'feature-branch', + 'main', + commit_message: 'Merge main into feature-branch' + ).and_return(nil) + + merge = Merge.new(config) + result = merge.merge_base_into_head + + expect(result[:success]).to be true + expect(result[:message]).to include('already up to date') + end + + it 'handles merge conflicts' do + expect(client).to receive(:merge).with( + 'owner/repo', + 'feature-branch', + 'main', + commit_message: 'Merge main into feature-branch' + ).and_raise(Octokit::Conflict.new) + + merge = Merge.new(config) + result = merge.merge_base_into_head + + expect(result[:success]).to be false + expect(result[:message]).to include('merge conflict detected') + end + + it 'handles other errors during merge' do + expect(client).to receive(:merge).with( + 'owner/repo', + 'feature-branch', + 'main', + commit_message: 'Merge main into feature-branch' + ).and_raise(StandardError.new('Network error')) + + merge = Merge.new(config) + result = merge.merge_base_into_head + + expect(result[:success]).to be false + expect(result[:message]).to include('Network error') + end + end +end