diff --git a/silent/tests/testdata/vu-group-semver-existing-pr.txt b/silent/tests/testdata/vu-group-semver-existing-pr.txt new file mode 100644 index 00000000000..cbca114a3ce --- /dev/null +++ b/silent/tests/testdata/vu-group-semver-existing-pr.txt @@ -0,0 +1,67 @@ +# A dependency whose only available update is a major bump is excluded from a +# minor/patch group by its update-types rules, so it must be raised as an +# individual pull request. +# +# This has to keep working once the group already has an open pull request: +# deferring the group to its own refresh job must not suppress the dependencies +# that the group's update-types rules reject. + +dependabot update -f input.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent +stdout -count=1 create_pull_request +pr-created expected-individual.json + +-- manifest.json -- +{ + "dependency-a": { "version": "1.2.3" }, + "dependency-b": { "version": "1.2.3" } +} + +-- expected-individual.json -- +{ + "dependency-a": { "version": "1.2.3" }, + "dependency-b": { "version": "2.0.0" } +} + +-- dependency-a -- +{ + "versions": [ + "1.2.3", + "1.2.4", + "1.2.5" + ] +} + +-- dependency-b -- +{ + "versions": [ + "1.2.3", + "2.0.0" + ] +} + +-- input.yml -- +job: + package-manager: "silent" + cooldown: + default-days: 0 + source: + directory: "/" + provider: example + hostname: 127.0.0.1 + api-endpoint: http://127.0.0.1/api/v3 + repo: dependabot/smoke-tests + dependency-groups: + - name: dev + rules: + update-types: + - minor + - patch + experiments: + individual-prs-for-semver-excluded-dependencies: true + existing-group-pull-requests: + - dependency-group-name: dev + pr-number: 123 + dependencies: + - dependency-name: dependency-a + dependency-version: 1.2.5 + directory: "/" diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb index c691a6f389f..968605b644e 100644 --- a/updater/lib/dependabot/dependency_snapshot.rb +++ b/updater/lib/dependabot/dependency_snapshot.rb @@ -4,6 +4,7 @@ require "base64" require "sorbet-runtime" +require "dependabot/experiments" require "dependabot/fetched_files" require "dependabot/file_parsers" require "dependabot/notices_helpers" @@ -112,45 +113,43 @@ def job_group @dependency_group_engine.find_group(name: T.must(job.dependency_group_to_refresh)) end + # `defer_update_types` is set by the callers that suppress individual updates for a whole group, i.e. + # the ones whose marking decides whether a dependency gets a pull request at all. sig do params( group: Dependabot::DependencyGroup, - excluding_dependencies: T::Hash[String, T::Set[String]] + excluding_dependencies: T::Hash[String, T::Set[String]], + defer_update_types: T::Boolean ) .void end - def mark_group_handled(group, excluding_dependencies = {}) + def mark_group_handled(group, excluding_dependencies = {}, defer_update_types: false) Dependabot.logger.info("Marking group '#{group.name}' as handled.") # When grouping by dependency name, we need to mark dependencies as handled # across ALL directories to prevent duplicate individual PRs group_by_name = group.group_by_dependency_name? + defer = defer_update_types && update_types_may_reject?(group) directories.each do |directory| @current_directory = directory - - # add the existing dependencies in the group so individual updates don't try to update them - dependencies_in_existing_prs = dependencies_in_existing_pr_for_group(group) - - dependencies_in_existing_prs = dependencies_in_existing_prs.filter do |dep| - # When grouping by name, include deps from all directories; otherwise filter by current directory - group_by_name || !dep.directory || dep.directory == directory - end - - # also add dependencies that might be in the group, as a rebase would add them; - # this avoids individual PR creation that immediately is superseded by a group PR supersede - current_dependencies = group.dependencies.map(&:name).reject do |dep| - excluding_dependencies[directory]&.include?(dep) - end - - add_handled_dependencies( - current_dependencies.concat( - dependencies_in_existing_prs.filter_map(&:name) - ) + mark_group_handled_in_current_directory( + group, excluding_dependencies, group_by_name: group_by_name, defer: defer ) end end + # The groups that may still claim this dependency once the semver level of its available update is + # known. An empty list means no group is waiting on it and it can be updated individually. + sig { params(dependency_name: String).returns(T::Array[Dependabot::DependencyGroup]) } + def groups_deferred_by_semver_rules(dependency_name) + assert_current_directory_set! + deferred = @deferred_group_dependencies[@current_directory] + return [] unless deferred + + deferred[dependency_name] || [] + end + sig { params(dependency_names: T.any(String, T::Array[String])).void } def add_handled_dependencies(dependency_names) assert_current_directory_set! @@ -204,6 +203,68 @@ def dependencies_in_existing_pr_for_group(group) private + sig do + params( + group: Dependabot::DependencyGroup, + excluding_dependencies: T::Hash[String, T::Set[String]], + group_by_name: T::Boolean, + defer: T::Boolean + ) + .void + end + def mark_group_handled_in_current_directory(group, excluding_dependencies, group_by_name:, defer:) + directory = @current_directory + + # add the existing dependencies in the group so individual updates don't try to update them + dependencies_in_existing_prs = dependencies_in_existing_pr_for_group(group).filter do |dep| + # When grouping by name, include deps from all directories; otherwise filter by current directory + group_by_name || !dep.directory || dep.directory == directory + end + + # also add dependencies that might be in the group, as a rebase would add them; + # this avoids individual PR creation that immediately is superseded by a group PR supersede + current_dependencies = group.dependencies.map(&:name).reject do |dep| + excluding_dependencies[directory]&.include?(dep) + end + + # A group that sets update-types only claims a dependency when the update available for it is at one + # of those semver levels, which is not known until an update checker has resolved the latest version. + # Marking those dependencies as handled here would silently drop the ones the group is going to + # reject, so the decision is deferred to the individual update run instead. + if defer + defer_to_group(group, current_dependencies) + current_dependencies = [] + end + + add_handled_dependencies( + current_dependencies.concat( + dependencies_in_existing_prs.filter_map(&:name) + ) + ) + end + + sig { params(group: Dependabot::DependencyGroup).returns(T::Boolean) } + def update_types_may_reject?(group) + return false unless Dependabot::Experiments.enabled?(:individual_prs_for_semver_excluded_dependencies) + + !group.update_types.nil? + end + + sig { params(group: Dependabot::DependencyGroup, dependency_names: T::Array[String]).void } + def defer_to_group(group, dependency_names) + return if dependency_names.empty? + + Dependabot.logger.info( + "Deferring to the update-types rules of group '#{group.name}': (#{dependency_names.join(', ')})." + ) + + deferred = (@deferred_group_dependencies[@current_directory] ||= {}) + dependency_names.uniq.each do |name| + groups = (deferred[name] ||= []) + groups << group unless groups.include?(group) + end + end + sig do params(job: Dependabot::Job, base_commit_sha: String, dependency_files: T::Array[Dependabot::DependencyFile]).void end @@ -214,6 +275,10 @@ def initialize(job:, base_commit_sha:, dependency_files:) # rubocop:disable Metr @base_commit_sha = base_commit_sha @dependency_files = dependency_files @handled_dependencies = T.let({}, T::Hash[String, T::Set[String]]) + @deferred_group_dependencies = T.let( + {}, + T::Hash[String, T::Hash[String, T::Array[Dependabot::DependencyGroup]]] + ) @current_directory = T.let("", String) @dependencies = T.let({}, T::Hash[String, T::Array[Dependabot::Dependency]]) diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index fb40e75582a..280f5f4dbf7 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -5,6 +5,7 @@ require "dependabot/dependency_change_builder" require "dependabot/updater/dependency_group_change_batch" +require "dependabot/updater/semver_grouping_rules" require "dependabot/workspace" require "dependabot/updater/security_update_helpers" require "dependabot/service" @@ -583,7 +584,6 @@ def all_versions_ignored?(dependency, checker) # then it should not be in the group, but be an individual PR, or in another group that fits it. # SemVer Grouping rules have to be applied after we have a checker, because we need to know the latest version. # Other rules are applied earlier in the process. - # rubocop:disable Metrics/AbcSize sig do params( group: Dependabot::DependencyGroup, @@ -593,109 +593,11 @@ def all_versions_ignored?(dependency, checker) .returns(T::Boolean) end def semver_rules_allow_grouping?(group, dependency, checker) - update_types = group.update_types - # There are no group rules defined, so this dependency can be included in the group. - return true unless update_types - - return cargo_semver_rules_allow_grouping?(group, dependency, checker) if job.package_manager == "cargo" - - version_class = Dependabot::Utils.version_class_for_package_manager(job.package_manager) - unless version_class.correct?(dependency.version.to_s) && version_class.correct?(checker.latest_version) - return false - end - - version = version_class.new(dependency.version.to_s) - latest_version = version_class.new(checker.latest_version) - - # Not every version class implements .major, .minor, .patch so we calculate it here from the segments - latest = semver_segments(latest_version) - current = semver_segments(version) - # Ensure that semver components are of the same type and can be compared with each other. - return false unless %i(major minor patch).all? { |k| current[k].instance_of?(latest[k].class) } - - return update_types.include?("major") if T.must(latest[:major]) > T.must(current[:major]) - return update_types.include?("minor") if T.must(latest[:minor]) > T.must(current[:minor]) - return update_types.include?("patch") if T.must(latest[:patch]) > T.must(current[:patch]) - - # some ecosystems don't do semver exactly, so anything lower gets individual for now - false - end - # rubocop:enable Metrics/AbcSize - - sig do - params( - group: Dependabot::DependencyGroup, - dependency: Dependabot::Dependency, - checker: Dependabot::UpdateCheckers::Base - ).returns(T::Boolean) - end - def cargo_semver_rules_allow_grouping?(group, dependency, checker) - case dependency.metadata[:all_versions] - when Array - return cargo_locked_line_updates_allowed?(group, dependency, checker) - end - - version_class = Dependabot::Utils.version_class_for_package_manager("cargo") - latest_version = checker.latest_version - return false unless version_class.correct?(dependency.version.to_s) && version_class.correct?(latest_version) - - cargo_update_type_allowed?( - group, - version_class.new(dependency.version.to_s), - version_class.new(latest_version) - ) - end - - sig do - params( - group: Dependabot::DependencyGroup, - dependency: Dependabot::Dependency, - checker: Dependabot::UpdateCheckers::Base - ).returns(T::Boolean) - end - def cargo_locked_line_updates_allowed?(group, dependency, checker) - requirements = requirements_to_unlock(checker) - return false if requirements == :update_not_possible - - updates = checker.updated_dependencies(requirements_to_unlock: requirements) - .select { |updated| updated.name.casecmp?(dependency.name) } - return false if updates.empty? - - version_class = Dependabot::Utils.version_class_for_package_manager("cargo") - updates.all? do |updated| - previous_version = updated.previous_version - version = updated.version - next false unless previous_version && version - next false unless version_class.correct?(previous_version) && version_class.correct?(version) - - cargo_update_type_allowed?( - group, - version_class.new(previous_version), - version_class.new(version) - ) + SemverGroupingRules.new(job: job).allow_grouping?(group, dependency, checker) do + requirements_to_unlock(checker) end end - sig { params(version: Gem::Version).returns(T::Hash[Symbol, Integer]) } - def semver_segments(version) - { - major: version.segments[0] || 0, - minor: version.segments[1] || 0, - patch: version.segments[2] || 0 - } - end - - sig { params(group: Dependabot::DependencyGroup, version: Gem::Version, latest_version: Gem::Version).returns(T::Boolean) } - def cargo_update_type_allowed?(group, version, latest_version) - return true unless Dependabot::Cargo::Version.respond_to?(:update_type) - - actual_update_type = Dependabot::Cargo::Version.update_type(version.to_s, latest_version.to_s) - group_update_types = group.update_types - return true unless group_update_types - - group_update_types.include?(actual_update_type) - end - sig { params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) } def requirements_to_unlock(checker) if !checker.requirements_unlocked_or_can_be? 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 d5e27fbd13d..80e67f9eec6 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -96,7 +96,7 @@ def run_grouped_dependency_updates Dependabot.logger.info( "Deferring creation of a new pull request. The existing pull request will update in a separate job." ) - dependency_snapshot.mark_group_handled(group) + dependency_snapshot.mark_group_handled(group, defer_update_types: true) next end @@ -106,7 +106,7 @@ def run_grouped_dependency_updates groups_without_pr.each do |group| dependency_change = run_grouped_update_for(group) # The update failed, add the suspected dependencies to the handled list so they don't update individually. - dependency_snapshot.mark_group_handled(group) if dependency_change.nil? + dependency_snapshot.mark_group_handled(group, defer_update_types: true) if dependency_change.nil? end end diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index e8582256233..7d35ffeff78 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true require "dependabot/updater/security_update_helpers" +require "dependabot/updater/semver_grouping_rules" require "dependabot/notices" # This class implements our strategy for iterating over all of the dependencies @@ -131,6 +132,12 @@ def check_and_create_pull_request(dependency) return if all_versions_ignored?(dependency, checker) return log_up_to_date(dependency) if checker.up_to_date? + if (group = group_claiming_dependency(dependency, checker)) + return Dependabot.logger.info( + "Skipping #{dependency.name} as its update is covered by the '#{group.name}' group" + ) + end + if pr_exists_for_latest_version?(checker) latest_version = checker.latest_version&.to_s return false if latest_version.nil? @@ -314,6 +321,25 @@ def existing_pull_request(updated_dependencies) created_pull_requests.find { |pr| pr == new_pr } end + # A dependency reaches this operation while a group still has a claim on it when that group was + # skipped without resolving any versions, e.g. because it already has an open pull request. Now + # that the latest version is known we can apply the group's update-types rules: if they accept + # this update the group's own job will raise it, so no individual pull request is opened. If they + # reject it the group will never include it, and this is the only chance it has to be updated. + sig do + params(dependency: Dependabot::Dependency, checker: Dependabot::UpdateCheckers::Base) + .returns(T.nilable(Dependabot::DependencyGroup)) + end + def group_claiming_dependency(dependency, checker) + groups = dependency_snapshot.groups_deferred_by_semver_rules(dependency.name) + return nil if groups.empty? + + semver_grouping_rules = SemverGroupingRules.new(job: job) + groups.find do |group| + semver_grouping_rules.allow_grouping?(group, dependency, checker) { requirements_to_unlock(checker) } + end + end + sig { params(checker: Dependabot::UpdateCheckers::Base).returns(Symbol) } def requirements_to_unlock(checker) if !checker.requirements_unlocked_or_can_be? diff --git a/updater/lib/dependabot/updater/semver_grouping_rules.rb b/updater/lib/dependabot/updater/semver_grouping_rules.rb new file mode 100644 index 00000000000..63d79f1c573 --- /dev/null +++ b/updater/lib/dependabot/updater/semver_grouping_rules.rb @@ -0,0 +1,156 @@ +# typed: strong +# frozen_string_literal: true + +require "sorbet-runtime" + +require "dependabot/dependency" +require "dependabot/dependency_group" +require "dependabot/utils" + +module Dependabot + class Updater + # Applies a dependency group's "update-types" rules to a single dependency. + # + # These rules can only be evaluated once an update checker has resolved the latest version, because + # they compare the semver level of the update that is actually available against the levels the group + # accepts. Every other group rule is applied earlier, while the group is being assembled. + # + # The caller supplies the "requirements to unlock" calculation as a block so it is only performed for + # the ecosystems that need it, since it is an expensive operation. + class SemverGroupingRules + extend T::Sig + + sig { params(job: Dependabot::Job).void } + def initialize(job:) + @job = job + end + + # This method applies "SemVer Grouping" rules: if the latest update is greater than the update-types, + # then it should not be in the group, but be an individual PR, or in another group that fits it. + # SemVer Grouping rules have to be applied after we have a checker, because we need to know the latest version. + # Other rules are applied earlier in the process. + # rubocop:disable Metrics/AbcSize + sig do + params( + group: Dependabot::DependencyGroup, + dependency: Dependabot::Dependency, + checker: Dependabot::UpdateCheckers::Base, + requirements_to_unlock: T.proc.returns(Symbol) + ) + .returns(T::Boolean) + end + def allow_grouping?(group, dependency, checker, &requirements_to_unlock) + update_types = group.update_types + # There are no group rules defined, so this dependency can be included in the group. + return true unless update_types + + if job.package_manager == "cargo" + return cargo_allow_grouping?(group, dependency, checker, requirements_to_unlock) + end + + version_class = Dependabot::Utils.version_class_for_package_manager(job.package_manager) + unless version_class.correct?(dependency.version.to_s) && version_class.correct?(checker.latest_version) + return false + end + + version = version_class.new(dependency.version.to_s) + latest_version = version_class.new(checker.latest_version) + + # Not every version class implements .major, .minor, .patch so we calculate it here from the segments + latest = semver_segments(latest_version) + current = semver_segments(version) + # Ensure that semver components are of the same type and can be compared with each other. + return false unless %i(major minor patch).all? { |k| current[k].instance_of?(latest[k].class) } + + return update_types.include?("major") if T.must(latest[:major]) > T.must(current[:major]) + return update_types.include?("minor") if T.must(latest[:minor]) > T.must(current[:minor]) + return update_types.include?("patch") if T.must(latest[:patch]) > T.must(current[:patch]) + + # some ecosystems don't do semver exactly, so anything lower gets individual for now + false + end + # rubocop:enable Metrics/AbcSize + + private + + sig { returns(Dependabot::Job) } + attr_reader :job + + sig do + params( + group: Dependabot::DependencyGroup, + dependency: Dependabot::Dependency, + checker: Dependabot::UpdateCheckers::Base, + requirements_to_unlock: T.proc.returns(Symbol) + ).returns(T::Boolean) + end + def cargo_allow_grouping?(group, dependency, checker, requirements_to_unlock) + case dependency.metadata[:all_versions] + when Array + return cargo_locked_line_updates_allowed?(group, dependency, checker, requirements_to_unlock) + end + + version_class = Dependabot::Utils.version_class_for_package_manager("cargo") + latest_version = checker.latest_version + return false unless version_class.correct?(dependency.version.to_s) && version_class.correct?(latest_version) + + cargo_update_type_allowed?( + group, + version_class.new(dependency.version.to_s), + version_class.new(latest_version) + ) + end + + sig do + params( + group: Dependabot::DependencyGroup, + dependency: Dependabot::Dependency, + checker: Dependabot::UpdateCheckers::Base, + requirements_to_unlock: T.proc.returns(Symbol) + ).returns(T::Boolean) + end + def cargo_locked_line_updates_allowed?(group, dependency, checker, requirements_to_unlock) + requirements = requirements_to_unlock.call + return false if requirements == :update_not_possible + + updates = checker.updated_dependencies(requirements_to_unlock: requirements) + .select { |updated| updated.name.casecmp?(dependency.name) } + return false if updates.empty? + + version_class = Dependabot::Utils.version_class_for_package_manager("cargo") + updates.all? do |updated| + previous_version = updated.previous_version + version = updated.version + next false unless previous_version && version + next false unless version_class.correct?(previous_version) && version_class.correct?(version) + + cargo_update_type_allowed?( + group, + version_class.new(previous_version), + version_class.new(version) + ) + end + end + + sig { params(version: Gem::Version).returns(T::Hash[Symbol, Integer]) } + def semver_segments(version) + { + major: version.segments[0] || 0, + minor: version.segments[1] || 0, + patch: version.segments[2] || 0 + } + end + + sig { params(group: Dependabot::DependencyGroup, version: Gem::Version, latest_version: Gem::Version).returns(T::Boolean) } + def cargo_update_type_allowed?(group, version, latest_version) + return true unless Dependabot::Cargo::Version.respond_to?(:update_type) + + actual_update_type = Dependabot::Cargo::Version.update_type(version.to_s, latest_version.to_s) + group_update_types = group.update_types + return true unless group_update_types + + group_update_types.include?(actual_update_type) + end + end + end +end diff --git a/updater/spec/dependabot/dependency_snapshot_spec.rb b/updater/spec/dependabot/dependency_snapshot_spec.rb index e0123466924..eff8f1f8182 100644 --- a/updater/spec/dependabot/dependency_snapshot_spec.rb +++ b/updater/spec/dependabot/dependency_snapshot_spec.rb @@ -109,6 +109,7 @@ let(:unsupported_error_enabled) { false } before do + allow(Dependabot::Experiments).to receive(:enabled?).and_call_original allow(Dependabot::Experiments).to receive(:enabled?) .with(:bundler_v1_unsupported_error) .and_return(unsupported_error_enabled) @@ -455,6 +456,75 @@ expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) end + context "when the group restricts the update types it accepts" do + let(:dependency_groups) do + [ + { + "name" => "group-a", + "rules" => { + "patterns" => ["dummy-pkg-*"], + "exclude-patterns" => ["dummy-pkg-b"], + "update-types" => %w(minor patch) + } + } + ] + end + + let(:existing_group_pull_requests) { [] } + let(:experiment_enabled) { true } + + before do + Dependabot::Experiments.register(:individual_prs_for_semver_excluded_dependencies, experiment_enabled) + end + + it "defers the dependencies to the group rather than marking them handled" do + snapshot = create_dependency_snapshot + group = snapshot.groups.first + snapshot.mark_group_handled(group, defer_update_types: true) + + %w(/foo /bar).each do |directory| + snapshot.current_directory = directory + + expect(snapshot.handled_dependencies).to be_empty + expect(snapshot.groups_deferred_by_semver_rules("dummy-pkg-a")).to eq([group]) + expect(snapshot.ungrouped_dependencies.map(&:name)).to include("dummy-pkg-a") + end + end + + it "marks the dependencies handled when the caller does not defer" do + snapshot = create_dependency_snapshot + snapshot.mark_group_handled(snapshot.groups.first) + snapshot.current_directory = "/foo" + + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + expect(snapshot.groups_deferred_by_semver_rules("dummy-pkg-a")).to be_empty + end + + it "does not defer dependencies the group never claimed" do + snapshot = create_dependency_snapshot + snapshot.mark_group_handled(snapshot.groups.first, defer_update_types: true) + snapshot.current_directory = "/foo" + + expect(snapshot.groups_deferred_by_semver_rules("dummy-pkg-b")).to be_empty + end + + context "when the experiment is disabled" do + let(:experiment_enabled) { false } + + it "marks the dependencies handled" do + snapshot = create_dependency_snapshot + snapshot.mark_group_handled(snapshot.groups.first, defer_update_types: true) + + %w(/foo /bar).each do |directory| + snapshot.current_directory = directory + + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + expect(snapshot.groups_deferred_by_semver_rules("dummy-pkg-a")).to be_empty + end + end + end + end + context "when there are no existing group pull requests" do let(:existing_group_pull_requests) { [] } diff --git a/updater/spec/dependabot/updater/operations/group_update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/group_update_all_versions_spec.rb index 67798ebc8d3..60db543cf34 100644 --- a/updater/spec/dependabot/updater/operations/group_update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/group_update_all_versions_spec.rb @@ -350,7 +350,7 @@ it "skips the group and marks it as handled" do expect(mock_create_group_update).not_to receive(:perform) - expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group) + expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group, defer_update_types: true) perform end @@ -414,7 +414,7 @@ it "skips creating a new PR" do expect(mock_create_group_update).not_to receive(:perform) - expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group) + expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group, defer_update_types: true) perform end end @@ -441,7 +441,7 @@ it "skips creating a new PR" do expect(mock_create_group_update).not_to receive(:perform) - expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group) + expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group, defer_update_types: true) perform end end @@ -499,7 +499,7 @@ it "treats the existing PR as a match" do expect(mock_create_group_update).not_to receive(:perform) - expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group) + expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group, defer_update_types: true) perform end end @@ -510,7 +510,7 @@ end it "marks the group as handled" do - expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group) + expect(dependency_snapshot).to receive(:mark_group_handled).with(dependency_group, defer_update_types: true) perform end end diff --git a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb index e25110522fb..43cb7ff4604 100644 --- a/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb +++ b/updater/spec/dependabot/updater/operations/update_all_versions_spec.rb @@ -472,6 +472,51 @@ update_all_versions.send(:check_and_create_pull_request, dependency) end end + + context "when a group is waiting on the dependency's update type" do + let(:group) do + Dependabot::DependencyGroup.new(name: "minor-and-patch", rules: { "update-types" => %w(minor patch) }) + end + + before do + allow(stub_update_checker).to receive_messages( + up_to_date?: false, + requirements_unlocked_or_can_be?: true, + updated_dependencies: [dependency] + ) + allow(update_all_versions).to receive_messages( + all_versions_ignored?: false, + existing_pull_request: false + ) + allow(Dependabot::DependencyChangeBuilder).to receive(:create_from).and_return(stub_dependency_change) + allow(dependency_snapshot).to receive(:groups_deferred_by_semver_rules) + .with("dummy-pkg-a").and_return([group]) + end + + context "when the group's update-types reject the available update" do + before do + # The dependency is at 4.0.0, so this is a major update the group does not accept. + allow(stub_update_checker).to receive(:latest_version).and_return(Gem::Version.new("5.0.0")) + end + + it "creates an individual pull request" do + expect(update_all_versions).to receive(:create_pull_request).with(stub_dependency_change) + update_all_versions.send(:check_and_create_pull_request, dependency) + end + end + + context "when the group's update-types accept the available update" do + before do + # The dependency is at 4.0.0, so this is a minor update the group will pick up itself. + allow(stub_update_checker).to receive(:latest_version).and_return(Gem::Version.new("4.1.0")) + end + + it "does not create an individual pull request" do + expect(update_all_versions).not_to receive(:create_pull_request) + update_all_versions.send(:check_and_create_pull_request, dependency) + end + end + end end describe "#update_checker_for" do