Merge pull request #7365 from dependabot/brrygrdn/group-branch-names-…

…are-deterministic

[Grouped Updates] Generate deterministic branch names based on content

common/lib/dependabot/pull_request_creator/branch_namer/dependency_group_strategy.rb

@@ -21,7 +21,7 @@ def initialize(dependencies:, files:, target_branch:, dependency_group:,
         # fixed-length name, so we can punt on handling truncation until
         # we determine the strict validation rules for names
         def new_branch_name
-          File.join(prefixes, timestamped_group_name).gsub("/", separator)
+          File.join(prefixes, group_name_with_dependency_digest).gsub("/", separator)
         end
 
         private
@@ -37,11 +37,20 @@ def prefixes
           ].compact
         end
 
-        # When superseding a grouped update pull request, we will have a period
-        # of time when there are two branches for the group so we use a timestamp
-        # to avoid collisions.
-        def timestamped_group_name
-          "#{dependency_group.name}-#{Time.now.utc.to_i}"
+        # Group pull requests will generally include too many dependencies to include
+        # in the branch name, but we rely on branch names being deterministic for a
+        # given set of dependency changes.
+        #
+        # Let's append a short hash digest of the dependency changes so that we can
+        # meet this guarantee.
+        def group_name_with_dependency_digest
+          "#{dependency_group.name}-#{dependency_digest}"
+        end
+
+        def dependency_digest
+          @dependency_digest ||= Digest::MD5.hexdigest(dependencies.map do |dependency|
+            "#{dependency.name}-#{dependency.removed? ? 'removed' : dependency.version}"
+          end.sort.join(",")).slice(0, 10)
         end
 
         def package_manager

common/spec/dependabot/pull_request_creator/branch_namer/dependency_group_strategy_spec.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require "digest"
+
 require "dependabot/dependency"
 require "dependabot/dependency_file"
 require "dependabot/dependency_group"
@@ -48,6 +50,70 @@
       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
+
+      it "generates a deterministic branch name for a given set of dependencies" do
+        branch_name = namer.new_branch_name
+        new_namer = described_class.new(
+          dependencies: dependencies,
+          files: [gemfile],
+          target_branch: target_branch,
+          separator: separator,
+          dependency_group: dependency_group
+        )
+        sleep 1 # ensure the timestamp changes
+        expect(new_namer.new_branch_name).to eql(branch_name)
+      end
+
+      it "generates a different branch name for a different set of dependencies for the same group" do
+        removed_dependency = Dependabot::Dependency.new(
+          name: "old_business",
+          version: "1.4.0",
+          previous_version: "1.4.0",
+          package_manager: "bundler",
+          requirements: {},
+          previous_requirements: {},
+          removed: true
+        )
+
+        new_namer = described_class.new(
+          dependencies: [dependency, removed_dependency],
+          files: [gemfile],
+          target_branch: target_branch,
+          separator: separator,
+          dependency_group: dependency_group
+        )
+        expect(new_namer.new_branch_name).not_to eql(namer.new_branch_name)
+      end
+
+      it "generates the same branch name regardless of the order of dependencies" do
+        removed_dependency = Dependabot::Dependency.new(
+          name: "old_business",
+          version: "1.4.0",
+          previous_version: "1.4.0",
+          package_manager: "bundler",
+          requirements: {},
+          previous_requirements: {},
+          removed: true
+        )
+
+        forward_namer = described_class.new(
+          dependencies: [dependency, removed_dependency],
+          files: [gemfile],
+          target_branch: target_branch,
+          separator: separator,
+          dependency_group: dependency_group
+        )
+
+        backward_namer = described_class.new(
+          dependencies: [removed_dependency, dependency],
+          files: [gemfile],
+          target_branch: target_branch,
+          separator: separator,
+          dependency_group: dependency_group
+        )
+
+        expect(forward_namer.new_branch_name).to eql(backward_namer.new_branch_name)
+      end
     end
 
     context "with a custom separator" do