diff --git a/common/lib/dependabot/dependency_group.rb b/common/lib/dependabot/dependency_group.rb index f6163ae97bc..0bd4e641794 100644 --- a/common/lib/dependabot/dependency_group.rb +++ b/common/lib/dependabot/dependency_group.rb @@ -2,10 +2,11 @@ module Dependabot class DependencyGroup - attr_reader :name + attr_reader :name, :rules - def initialize(name) + def initialize(name:, rules:) @name = name + @rules = rules end end end diff --git a/common/lib/dependabot/pull_request_creator.rb b/common/lib/dependabot/pull_request_creator.rb index 4b86e8ec45b..bbfa42bf340 100644 --- a/common/lib/dependabot/pull_request_creator.rb +++ b/common/lib/dependabot/pull_request_creator.rb @@ -49,7 +49,7 @@ def initialize(cause, pull_request) :commit_message_options, :vulnerabilities_fixed, :reviewers, :assignees, :milestone, :branch_name_separator, :branch_name_prefix, :branch_name_max_length, :github_redirection_service, - :custom_headers, :provider_metadata + :custom_headers, :provider_metadata, :dependency_group def initialize(source:, base_commit:, dependencies:, files:, credentials:, pr_message_header: nil, pr_message_footer: nil, @@ -61,7 +61,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, automerge_candidate: false, github_redirection_service: DEFAULT_GITHUB_REDIRECTION_SERVICE, custom_headers: nil, require_up_to_date_base: false, - provider_metadata: {}, message: nil) + provider_metadata: {}, message: nil, dependency_group: nil) @dependencies = dependencies @source = source @base_commit = base_commit @@ -87,6 +87,7 @@ def initialize(source:, base_commit:, dependencies:, files:, credentials:, @require_up_to_date_base = require_up_to_date_base @provider_metadata = provider_metadata @message = message + @dependency_group = dependency_group check_dependencies_have_previous_version end @@ -235,7 +236,7 @@ def branch_namer dependencies: dependencies, files: files, target_branch: source.branch, - dependency_group: nil, + dependency_group: dependency_group, separator: branch_name_separator, prefix: branch_name_prefix, max_length: branch_name_max_length diff --git a/common/lib/dependabot/pull_request_creator/branch_namer.rb b/common/lib/dependabot/pull_request_creator/branch_namer.rb index a81e1bd5fed..dcd3da49e43 100644 --- a/common/lib/dependabot/pull_request_creator/branch_namer.rb +++ b/common/lib/dependabot/pull_request_creator/branch_namer.rb @@ -5,6 +5,7 @@ require "dependabot/metadata_finders" require "dependabot/pull_request_creator" require "dependabot/pull_request_creator/branch_namer/solo_strategy" +require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy" module Dependabot class PullRequestCreator diff --git a/common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb b/common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb index 8b84dfa5fbf..7a2384615d5 100644 --- a/common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb +++ b/common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb @@ -15,13 +15,40 @@ def initialize(dependencies:, files:, target_branch:, dependency_group:, @max_length = max_length end + # FIXME: Incorporate max_length truncation once we allow user config + # + # For now, we are using a placeholder DependencyGroup with a + # fixed-length name, so we can punt on handling truncation until + # we determine the strict validation rules for names def new_branch_name - dependency_group.name + File.join(prefixes, dependency_group.name, prototype_suffix).gsub("/", separator) end private - attr_reader :dependency_group + attr_reader :dependencies, :dependency_group, :files, :target_branch, :separator, :prefix, :max_length + + def prefixes + [ + prefix, + package_manager, + directory, + target_branch + ].compact + end + + # FIXME: Remove once grouped PRs can supersede each other + def prototype_suffix + "prototype-#{Time.now.utc.to_i}" + end + + def package_manager + dependencies.first.package_manager + end + + def directory + files.first.directory.tr(" ", "-") + end end end end diff --git a/common/spec/dependabot/dependency_group_spec.rb b/common/spec/dependabot/dependency_group_spec.rb index d792c494aed..7a5f0e0a803 100644 --- a/common/spec/dependabot/dependency_group_spec.rb +++ b/common/spec/dependabot/dependency_group_spec.rb @@ -6,7 +6,7 @@ describe "#name" do it "returns the name" do my_dependency_group_name = "darren-from-work" - dependency_group = described_class.new(my_dependency_group_name) + dependency_group = described_class.new(name: my_dependency_group_name, rules: anything) expect(dependency_group.name).to eq(my_dependency_group_name) end diff --git a/common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb b/common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb index 4b64b64b810..631d42d908f 100644 --- a/common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb +++ b/common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb @@ -1,19 +1,93 @@ # frozen_string_literal: true +require "dependabot/dependency" +require "dependabot/dependency_file" +require "dependabot/dependency_group" require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy" RSpec.describe Dependabot::PullRequestCreator::BranchNamer::DependencyGroupStrategy do + subject(:namer) do + described_class.new( + dependencies: dependencies, + files: [gemfile], + target_branch: target_branch, + separator: separator, + dependency_group: dependency_group + ) + end + + let(:dependencies) { [dependency] } + let(:dependency) do + Dependabot::Dependency.new( + name: "business", + version: "1.5.0", + previous_version: "1.4.0", + package_manager: "bundler", + requirements: {}, + previous_requirements: {} + ) + end + let(:gemfile) do + Dependabot::DependencyFile.new( + name: "Gemfile", + content: "anything", + directory: directory + ) + end + + let(:dependency_group) do + Dependabot::DependencyGroup.new(name: "my-dependency-group", rules: anything) + end + describe "#new_branch_name" do - it "returns the name of the dependency group" do - dependency_group = double("DependencyGroup", name: "my_dependency_group") - strategy = described_class.new( - dependencies: [], - files: [], - target_branch: "main", - dependency_group: dependency_group - ) - - expect(strategy.new_branch_name).to eq(dependency_group.name) + context "with defaults for separator, target branch and files in the root directory" do + let(:directory) { "/" } + let(:target_branch) { nil } + let(:separator) { "/" } + + it "returns the name of the dependency group prefixed correctly" do + expect(namer.new_branch_name).to start_with("dependabot/bundler/my-dependency-group") + end + end + + context "with a custom separator" do + let(:directory) { "/" } + let(:target_branch) { nil } + let(:separator) { "_" } + + it "returns the name of the dependency group prefixed correctly" do + expect(namer.new_branch_name).to start_with("dependabot_bundler_my-dependency-group") + end + end + + context "for files in a non-root directory" do + let(:directory) { "rails app/" } # let's make sure we deal with spaces too + let(:target_branch) { nil } + let(:separator) { "/" } + + it "returns the name of the dependency group prefixed correctly" do + expect(namer.new_branch_name).to start_with("dependabot/bundler/rails-app/my-dependency-group") + end + end + + context "targeting a branch" do + let(:directory) { "/" } + let(:target_branch) { "develop" } + let(:separator) { "/" } + + it "returns the name of the dependency group prefixed correctly" do + expect(namer.new_branch_name).to start_with("dependabot/bundler/develop/my-dependency-group") + end + end + + context "for files in a non-root directory targetting a branch" do + let(:directory) { "rails-app/" } + let(:target_branch) { "develop" } + let(:separator) { "_" } + + it "returns the name of the dependency group prefixed correctly" do + expect(namer.new_branch_name).to start_with("dependabot_bundler_rails-app_develop_my-dependency-group") + end end end end diff --git a/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb b/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb index 3c37ff9be29..9cc965f633a 100644 --- a/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb +++ b/common/spec/dependabot/pull_request_creator/branch_namer_spec.rb @@ -5,8 +5,6 @@ require "dependabot/dependency" require "dependabot/dependency_file" require "dependabot/pull_request_creator/branch_namer" -require "dependabot/pull_request_creator/branch_namer/solo_strategy" -require "dependabot/pull_request_creator/branch_namer/dependency_group_strategy" RSpec.describe Dependabot::PullRequestCreator::BranchNamer do subject(:namer) do diff --git a/common/spec/dependabot/pull_request_creator_spec.rb b/common/spec/dependabot/pull_request_creator_spec.rb index bcffc7905ae..3362a79d13a 100644 --- a/common/spec/dependabot/pull_request_creator_spec.rb +++ b/common/spec/dependabot/pull_request_creator_spec.rb @@ -4,6 +4,7 @@ require "spec_helper" require "dependabot/dependency" require "dependabot/dependency_file" +require "dependabot/dependency_group" require "dependabot/pull_request_creator" require "dependabot/pull_request_creator/message" @@ -354,5 +355,54 @@ creator.create end end + + context "with a dependency group" do + let(:dependency_group) { Dependabot::DependencyGroup.new(name: "all-the-things", rules: anything) } + let(:source) { Dependabot::Source.new(provider: "github", repo: "gc/bp") } + let(:dummy_creator) { instance_double(described_class::Github) } + + subject(:creator_with_group) do + described_class.new( + source: source, + base_commit: base_commit, + dependencies: dependencies, + files: files, + credentials: credentials, + custom_labels: custom_labels, + reviewers: reviewers, + assignees: assignees, + milestone: milestone, + author_details: author_details, + signature_key: signature_key, + provider_metadata: provider_metadata, + dependency_group: dependency_group + ) + end + + it "delegates to PullRequestCreator::Github with correct params" do + expect(described_class::Github). + to receive(:new). + with( + source: source, + branch_name: start_with("dependabot/bundler/all-the-things/prototype-"), + base_commit: base_commit, + credentials: credentials, + files: files, + commit_message: "Commit msg", + pr_description: "PR msg", + pr_name: "PR name", + author_details: author_details, + signature_key: signature_key, + custom_headers: nil, + labeler: instance_of(described_class::Labeler), + reviewers: reviewers, + assignees: assignees, + milestone: milestone, + require_up_to_date_base: false + ).and_return(dummy_creator) + expect(dummy_creator).to receive(:create) + creator_with_group.create + end + end end end diff --git a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb index c7b4935d4f0..dabb9258b44 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -37,7 +37,7 @@ def initialize(service:, job:, dependency_snapshot:, error_handler:) @dependency_snapshot = dependency_snapshot @error_handler = error_handler # This is a placeholder for a real rule object obtained from config in future - @dependency_group = Dependabot::DependencyGroup.new(name: GROUP_NAME_PLACEHOLDER) + @dependency_group = Dependabot::DependencyGroup.new(name: GROUP_NAME_PLACEHOLDER, rules: []) end def perform