Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handling of origin to method point_to_same_commit? #590

Merged
merged 3 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ _None_

### Internal Changes

_None_
- Added the handling of remote to `GitHelper::point_to_same_commit?` [#590]

## 12.0.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,7 @@ def self.create_backmerge_pr(token:, repository:, title:, head_branch:, base_bra
# if there's a callback, make sure it didn't switch branches
other_action.ensure_git_branch(branch: "^#{intermediate_branch}/") unless intermediate_branch_created_callback.nil?

base_branch_ref = base_branch.start_with?('origin/') ? base_branch : "origin/#{base_branch}"
head_branch_ref = head_branch.start_with?('origin/') ? head_branch : "origin/#{head_branch}"

if Fastlane::Helper::GitHelper.point_to_same_commit?(base_branch_ref, head_branch_ref)
if Fastlane::Helper::GitHelper.point_to_same_commit?(base_branch, head_branch)
UI.error("No differences between #{head_branch} and #{base_branch}. Skipping PR creation.")
return nil
end
Expand Down
13 changes: 8 additions & 5 deletions lib/fastlane/plugin/wpmreleasetoolkit/helper/git_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,20 +174,23 @@ def self.fetch_all_tags
#
# @param ref1 [String] the first git reference to check.
# @param ref2 [String] the second git reference to check.
# @param remote_name [String] the name of the remote repository to use (default is 'origin').
# If nil or empty, no remote prefix will be used.
#
# @return [Boolean] true if the two references point to the same commit, false otherwise.
#
def self.point_to_same_commit?(ref1, ref2)
def self.point_to_same_commit?(ref1, ref2, remote_name: 'origin')
git_repo = Git.open(Dir.pwd)

ref1_full = remote_name.to_s.empty? ? ref1 : "#{remote_name}/#{ref1}"
ref2_full = remote_name.to_s.empty? ? ref2 : "#{remote_name}/#{ref2}"
begin
ref1_commit = git_repo.gcommit(ref1)
ref2_commit = git_repo.gcommit(ref2)
ref1_commit = git_repo.gcommit(ref1_full)
ref2_commit = git_repo.gcommit(ref2_full)
rescue StandardError => e
puts "Error: #{e.message}"
UI.error "Error fetching commits for #{ref1_full} and #{ref2_full}: #{e.message}"
return false
end

ref1_commit.sha == ref2_commit.sha
end

Expand Down
2 changes: 1 addition & 1 deletion spec/create_release_backmerge_pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def stub_expected_pull_requests(expected_backmerge_branches:, source_branch:, la

next if branch_exists_on_remote

allow(Fastlane::Helper::GitHelper).to receive(:point_to_same_commit?).with("origin/#{target_branch}", "origin/#{source_branch}").and_return(point_to_same_commit)
allow(Fastlane::Helper::GitHelper).to receive(:point_to_same_commit?).with(target_branch, source_branch).and_return(point_to_same_commit)

next if point_to_same_commit

Expand Down
14 changes: 7 additions & 7 deletions spec/git_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,37 +145,37 @@
end

it 'checks if a tag and a branch point to the same commit' do
same_commit = described_class.point_to_same_commit?('1.0', 'another-branch')
same_commit = described_class.point_to_same_commit?('1.0', 'another-branch', remote_name: nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test using remote_name: 'origin',

    it 'checks if commits between the same branch point to the same commit' do
      same_commit = described_class.point_to_same_commit?('feature-branch', 'feature-branch', remote_name: 'origin')
      expect(same_commit).to be true
    end

And got this failure:

  1) FastlaneCore::Helper::GitHelper point_to_same_commit?(ref1, ref2) checks if commits between the same branch point to the same commit
     Failure/Error: ref1_commit.sha == ref2_commit.sha

     Git::FailedError:
       git '--git-dir=/private/var/folders/dq/cdqxvx3s5ps75564rpmb_dc00000gn/T/d20240906-32427-fzffvl/.git' '--work-tree=/private/var/folders/dq/cdqxvx3s5ps75564rpmb_dc00000gn/T/d20240906-32427-fzffvl' '-c' 'core.quotePath=true' '-c' 'color.ui=false' 'rev-parse' 'origin/feature-branch'  2>&1
       status: pid 32722 exit 128
       output: "fatal: ambiguous argument 'origin/feature-branch': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\norigin/feature-branch\n"

I think that's expected because the local branches used in this test do not have a remote. Am I right?

What do you think of adding a note about this, e.g. at the start of the context (why not describe?) for this method.

# We cannot test the happy using a remote name because the repo we use for the tests does not have a remote.
let(:remote_name) { nil }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestions, thanks! Updated on 36cd7bc.

expect(same_commit).to be false
end

it 'checks if a tag and a branch that had a merge point to the same commit' do
same_commit = described_class.point_to_same_commit?('1.0', 'main')
same_commit = described_class.point_to_same_commit?('1.0', 'main', remote_name: nil)
expect(same_commit).to be false
end

it 'checks if a tag and a commit hash point to the same commit' do
same_commit = described_class.point_to_same_commit?('1.0', commit_hash(commit_message: 'commit D'))
same_commit = described_class.point_to_same_commit?('1.0', commit_hash(commit_message: 'commit D'), remote_name: nil)
expect(same_commit).to be false
end

it 'checks if a commit hash and a branch point to the same commit' do
same_commit = described_class.point_to_same_commit?(commit_hash(commit_message: 'commit B'), 'another-branch')
same_commit = described_class.point_to_same_commit?(commit_hash(commit_message: 'commit B'), 'another-branch', remote_name: nil)
expect(same_commit).to be false
end

it 'checks if commits between the same branch point to the same commit' do
same_commit = described_class.point_to_same_commit?('feature-branch', 'feature-branch')
same_commit = described_class.point_to_same_commit?('feature-branch', 'feature-branch', remote_name: nil)
expect(same_commit).to be true
end

it 'checks if commits between branches that have no difference point to the same commit' do
same_commit = described_class.point_to_same_commit?('another-branch', 'new-branch')
same_commit = described_class.point_to_same_commit?('another-branch', 'new-branch', remote_name: nil)
expect(same_commit).to be true
end

it 'raises error for a non-existent base_ref' do
expect { described_class.point_to_same_commit?('non-existent', 'main') }.to raise_error(StandardError)
expect { described_class.point_to_same_commit?('non-existent', 'main', remote_name: nil) }.to raise_error(StandardError)
end
end

Expand Down