Skip to content

Commit

Permalink
fixes as required
Browse files Browse the repository at this point in the history
  • Loading branch information
sachin-sandhu committed Nov 1, 2024
1 parent 40da87f commit 8d0b0cb
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 40 deletions.
27 changes: 1 addition & 26 deletions common/lib/dependabot/pull_request_creator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
require "dependabot/credential"

module Dependabot
# rubocop:disable Metrics/ClassLength
class PullRequestCreator
extend T::Sig

Expand Down Expand Up @@ -41,7 +40,7 @@ class NoHistoryInCommon < StandardError; end

class UnmergedPRExists < StandardError; end

class DuplicateBranchExists < StandardError; end
class BranchAlreadyExists < StandardError; end

class BaseCommitNotUpToDate < StandardError; end

Expand Down Expand Up @@ -243,12 +242,6 @@ def check_dependencies_have_previous_version
# then convert to that
sig { returns(T.untyped) }
def create
if trace_log?
Dependabot.logger.info(
"Dependabot::PullRequestCreator::create => var source.provider : #{source.provider}"
)
end

case source.provider
when "github" then github_creator.create
when "gitlab" then gitlab_creator.create
Expand Down Expand Up @@ -278,12 +271,6 @@ def require_up_to_date_base?

sig { returns(Dependabot::PullRequestCreator::Github) }
def github_creator
if trace_log?
Dependabot.logger.info(
"Dependabot::PullRequestCreator::create::github_creator"
)
end

Github.new(
source: source,
branch_name: branch_namer.new_branch_name,
Expand Down Expand Up @@ -411,12 +398,6 @@ def message

sig { returns(Dependabot::PullRequestCreator::BranchNamer) }
def branch_namer
if trace_log?
Dependabot.logger.info(
"Dependabot::PullRequestCreator::branch_namer"
)
end

@branch_namer ||= T.let(
BranchNamer.new(
dependencies: dependencies,
Expand Down Expand Up @@ -458,11 +439,5 @@ def includes_security_fixes?
def requirements_changed?(dependency)
(dependency.requirements - T.must(dependency.previous_requirements)).any?
end

sig { returns(T::Boolean) }
def trace_log?
Dependabot::Experiments.enabled?(:dedup_branch_names)
end
end
end
# rubocop:enable Metrics/ClassLength
18 changes: 8 additions & 10 deletions common/lib/dependabot/pull_request_creator/github.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ def initialize(source:, branch_name:, base_commit:, credentials:,

sig { returns(T.untyped) }
def create
if trace_log? && branch_exists?(branch_name)
if experiment_duplicate_branch? && branch_exists?(branch_name)
Dependabot.logger.info(
"Existing branch \"#{branch_name}\" found: raising DuplicateBranchExists exception."
"Existing branch \"#{branch_name}\" found. Pull request not created."
)
raise DuplicateBranchExists, "Duplicate branch #{branch_name} already exists"
raise BranchAlreadyExists, "Duplicate branch #{branch_name} already exists"
end

if branch_exists?(branch_name) && unmerged_pull_request_exists?
Expand All @@ -139,12 +139,10 @@ def require_up_to_date_base?
# rubocop:disable Metrics/PerceivedComplexity
sig { params(name: String).returns(T::Boolean) }
def branch_exists?(name)
if trace_log?
Dependabot.logger.info(
"Dependabot::PullRequestCreator::Github:branch_exists?. " \
"Name : #{name}. IsDuplicate: #{git_metadata_fetcher.ref_names.include?(name)}"
)
end
Dependabot.logger.debug(
"Dependabot::PullRequestCreator::Github:branch_exists?. " \
"Name : #{name}. IsDuplicate: #{git_metadata_fetcher.ref_names.include?(name)}"
)

git_metadata_fetcher.ref_names.include?(name)
rescue Dependabot::GitDependenciesNotReachable => e
Expand Down Expand Up @@ -596,7 +594,7 @@ def raise_custom_error(base_err, type, message)
end

sig { returns(T::Boolean) }
def trace_log?
def experiment_duplicate_branch?
Dependabot::Experiments.enabled?(:dedup_branch_names)
end
end
Expand Down
8 changes: 4 additions & 4 deletions common/spec/dependabot/pull_request_creator/github_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -613,16 +613,17 @@
end

context "when the branch already exists" do
let(:service_pack_response) { fixture("git", "upload_packs", "existing-branch") }

before do
service_pack_response.gsub!("heads/rubocop", "heads/#{branch_name}")
Dependabot::Experiments.register(:dedup_branch_names, true)
end

after do
Dependabot::Experiments.register(:dedup_branch_names, false)
end

context "when a PR to this branch doesn't exist" do
context "when the branch already exists" do
before do
url = "#{repo_api_url}/pulls?head=gocardless:#{branch_name}" \
"&state=all"
Expand All @@ -640,9 +641,8 @@

it "returns a suitable exception" do
expect { creator.create }
.to raise_error(Dependabot::PullRequestCreator::DuplicateBranchExists,
.to raise_error(Dependabot::PullRequestCreator::BranchAlreadyExists,
"Duplicate branch #{branch_name} already exists")
expect(WebMock).not_to have_requested(:post, "#{repo_api_url}/pulls")
end
end
end
Expand Down
Binary file not shown.

0 comments on commit 8d0b0cb

Please sign in to comment.