Add handling of origin to method point_to_same_commit?

[iangmaia]

What does it do?

Starting from the comment #587 (comment), I realized we already have checks and logic in place to make sure the head and base branches start with release/ and that it would make more sense to have any logic related to the point_to_same_commit? check locally vs. on remote in the method itself, so this PR updates point_to_same_commit? for that.

Checklist before requesting a review

• Run bundle exec rubocop to test for code style violations and recommendations
• Add Unit Tests (aka specs/*_spec.rb) if applicable
• Run bundle exec rspec to run the whole test suite and ensure all your tests pass
• Make sure you added an entry in the CHANGELOG.md file to describe your changes under the appropriate existing ### subsection of the existing ## Trunk section.
• If applicable, add an entry in the MIGRATION.md file to describe how the changes will affect the migration from the previous major version and what the clients will need to change and consider.

[mokagio]

Interesting. CI reports failures in tests that are stubbing point_to_same_commit? (a good case for avoiding stubs, however that's easier said than done in this repo...).

Example:

FastlaneCore::Helper::GitHelper received :point_to_same_commit? with unexpected arguments
  | expected: ("origin/release/30.6", "origin/release/30.7")
  | got: ("trunk", "release/30.7")
  | Please stub a default value first if message might be received with other args as well.

[iangmaia]

Interesting. CI reports failures in tests that are stubbing point_to_same_commit? (a good case for avoiding stubs, however that's easier said than done in this repo...).

Example:

FastlaneCore::Helper::GitHelper received :point_to_same_commit? with unexpected arguments
  | expected: ("origin/release/30.6", "origin/release/30.7")
  | got: ("trunk", "release/30.7")
  | Please stub a default value first if message might be received with other args as well.

True, I should've updated stub_expected_pull_requests with:

- 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)

It should be fixed now.

[mokagio]

Only a non-blocking suggestion for clarity in the tests.

[mokagio]

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 }

[iangmaia]

Great suggestions, thanks! Updated on 36cd7bc.

[mokagio]

Thanks for following up @iangmaia !