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

Teach updater how to do multi-directory version pull requests #8541

Merged
merged 14 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all 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 updater/lib/dependabot/dependency_change.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def should_replace_existing_pr?
# NOTE: Gradle, Maven and Nuget dependency names can be case-insensitive
# and the dependency name injected from a security advisory often doesn't
# match what users have specified in their manifest.
updated_dependencies.map { |x| x.name.downcase } != job.dependencies.map(&:downcase)
updated_dependencies.map { |x| x.name.downcase }.uniq.sort != job.dependencies.map(&:downcase).uniq.sort
jakecoffman marked this conversation as resolved.
Show resolved Hide resolved
end

def matches_existing_pr?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ def perform

Dependabot.logger.info("Starting update group for '#{group.name}'")

dependency_change = compile_all_dependency_changes_for(group)

if dependency_change.updated_dependencies.any?
Dependabot.logger.info("Creating a pull request for '#{group.name}'")
begin
Expand All @@ -64,6 +62,26 @@ def perform
:dependency_snapshot,
:error_handler,
:group

def dependency_change
return @dependency_change if defined?(@dependency_change)

if job.source.directories.nil?
@dependency_change = compile_all_dependency_changes_for(group)
else
dependency_changes = job.source.directories.map do |directory|
job.source.directory = directory
# Fixes not updating because it already updated in a previous group
dependency_snapshot.handled_dependencies.clear
compile_all_dependency_changes_for(group)
end

# merge the changes together into one
@dependency_change = dependency_changes.first
@dependency_change.merge_changes!(dependency_changes[1..-1]) if dependency_changes.count > 1
@dependency_change
end
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def perform # rubocop:disable Metrics/AbcSize
)
end

dependency_change = compile_all_dependency_changes_for(dependency_snapshot.job_group)
dependency_change

upsert_pull_request_with_error_handling(dependency_change, dependency_snapshot.job_group)
end
Expand All @@ -99,6 +99,26 @@ def perform # rubocop:disable Metrics/AbcSize
:service,
:dependency_snapshot,
:error_handler

def dependency_change
return @dependency_change if defined?(@dependency_change)

if job.source.directories.nil?
@dependency_change = compile_all_dependency_changes_for(dependency_snapshot.job_group)
else
dependency_changes = job.source.directories.map do |directory|
job.source.directory = directory
# Fixes not updating because it already updated in a previous group
dependency_snapshot.handled_dependencies.clear
compile_all_dependency_changes_for(dependency_snapshot.job_group)
end

# merge the changes together into one
@dependency_change = dependency_changes.first
@dependency_change.merge_changes!(dependency_changes[1..-1]) if dependency_changes.count > 1
@dependency_change
end
end
end
end
end
Expand Down
101 changes: 101 additions & 0 deletions updater/spec/dependabot/dependency_change_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,107 @@
end
end

describe "#should_replace_existing_pr" do
context "when not updating a pull request" do
let(:job) do
instance_double(Dependabot::Job, updating_a_pull_request?: false)
end

it "is false" do
expect(dependency_change.should_replace_existing_pr?).to be false
end
end

context "when updating a pull request with all dependencies matching" do
let(:job) do
instance_double(Dependabot::Job,
dependencies: ["business"],
updating_a_pull_request?: true)
end

it "returns false" do
expect(dependency_change.should_replace_existing_pr?).to be false
end
end

context "when updating a pull request with duplicate dependencies" do
let(:job) do
instance_double(Dependabot::Job,
dependencies: %w(business business),
updating_a_pull_request?: true)
end

it "returns false" do
expect(dependency_change.should_replace_existing_pr?).to be false
end
end

context "when updating a pull request with non-matching casing" do
let(:job) do
instance_double(Dependabot::Job,
dependencies: ["BuSiNeSS"],
updating_a_pull_request?: true)
end

it "returns false" do
expect(dependency_change.should_replace_existing_pr?).to be false
end
end

context "when updating a pull request with out of order dependencies" do
let(:job) do
instance_double(Dependabot::Job,
dependencies: %w(PkgB PkgA),
updating_a_pull_request?: true)
end

let(:updated_dependencies) do
[
Dependabot::Dependency.new(
name: "PkgA",
package_manager: "bundler",
version: "1.8.0",
previous_version: "1.7.0",
requirements: [
{ file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil }
],
previous_requirements: [
{ file: "Gemfile", requirement: "~> 1.7.0", groups: [], source: nil }
]
),
Dependabot::Dependency.new(
name: "PkgB",
package_manager: "bundler",
version: "1.8.0",
previous_version: "1.7.0",
requirements: [
{ file: "Gemfile", requirement: "~> 1.8.0", groups: [], source: nil }
],
previous_requirements: [
{ file: "Gemfile", requirement: "~> 1.7.0", groups: [], source: nil }
]
)
]
end

it "returns false" do
expect(dependency_change.should_replace_existing_pr?).to be false
end
end

context "when updating a pull request with different dependencies" do
let(:job) do
instance_double(Dependabot::Job,
dependencies: ["contoso"],
updating_a_pull_request?: true)
end

it "returns true" do
expect(dependency_change.should_replace_existing_pr?).to be true
end
end
end

describe "#grouped_update?" do
it "is false by default" do
expect(dependency_change.grouped_update?).to be false
Expand Down