From ee02a468967ef573550e536cabcc4554639d893d Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Tue, 2 Jan 2024 13:13:16 +0000 Subject: [PATCH] Fix dependency name edge-case A recent PR was auto-merged when it should not have been: https://github.com/alphagov/content-data-api/pull/1996 This is because the name of the dependency was not being matched by the regex (`\w` does not support hyphenated names). It therefore looked as though _NO_ dependencies were being updated, and it seemed therefore that there were no top-level dependencies being updated that weren't on the allowlist. govuk-dependabot-merger therefore gave the green light to auto-merge. Have now tweaked the regex to catch any kind of dependency name. This did break the `multiple_dependencies_commit` spec, because each of its dependencies is wrapped in backticks (as opposed to the single-dependency commits, which had no backticks), so the names weren't matching. I've now added an inline `gsub` to remove the backticks from the names, since they're never actually part of the dependency names. --- lib/pull_request.rb | 5 ++-- spec/lib/pull_request_spec.rb | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/lib/pull_request.rb b/lib/pull_request.rb index 0603d4e..8528ac9 100644 --- a/lib/pull_request.rb +++ b/lib/pull_request.rb @@ -139,10 +139,9 @@ def tell_dependency_manager_what_dependencies_are_allowed end def tell_dependency_manager_what_dependabot_is_changing - dependency_updates = commit_message.scan(/(?:Bump|Updates) `?(\w+)`? from (\d+\.\d+\.\d+) to (\d+\.\d+\.\d+)/) - - mentioned_dependencies = dependency_updates.to_h { |name, from_version, to_version| [name, { from_version:, to_version: }] } + dependency_updates = commit_message.scan(/(?:Bump|Updates) (.+) from (\d+\.\d+\.\d+) to (\d+\.\d+\.\d+)/) + mentioned_dependencies = dependency_updates.to_h { |name, from_version, to_version| [name.gsub(/`/m, ""), { from_version:, to_version: }] } lines_removed = gemfile_lock_changes.scan(/^-\s+([a-z\-_]+) \(([0-9.]+)\)$/) lines_added = gemfile_lock_changes.scan(/^\+\s+([a-z\-_]+) \(([0-9.]+)\)$/) diff --git a/spec/lib/pull_request_spec.rb b/spec/lib/pull_request_spec.rb index 6fc5c94..0ce0e1e 100644 --- a/spec/lib/pull_request_spec.rb +++ b/spec/lib/pull_request_spec.rb @@ -23,6 +23,26 @@ def single_dependency_commit TEXT end + def single_external_dependency_commit + <<~TEXT + Bump shoulda-matchers from 5.3.0 to 6.0.0 + + Bumps [shoulda-matchers](https://github.com/thoughtbot/shoulda-matchers) from 5.3.0 to 6.0.0. + - [Release notes](https://github.com/thoughtbot/shoulda-matchers/releases) + - [Changelog](https://github.com/thoughtbot/shoulda-matchers/blob/main/CHANGELOG.md) + - [Commits](thoughtbot/shoulda-matchers@v5.3.0...v6.0.0) + + --- + updated-dependencies: + - dependency-name: shoulda-matchers + dependency-type: direct:development + update-type: version-update:semver-major + ... + + Signed-off-by: dependabot[bot] + TEXT + end + def multiple_dependencies_commit <<~TEXT Bump rack, rails and govuk_sidekiq @@ -512,6 +532,31 @@ def create_mock_dependency_manager ) pull_request.tell_dependency_manager_what_dependabot_is_changing end + + it "supports hyphenated names" do + dependency_manager = double("DependencyManager") + api_response = "foo" + pull_request = PullRequest.new(api_response, dependency_manager) + allow(pull_request).to receive(:commit_message).and_return(single_external_dependency_commit) + allow(pull_request).to receive(:gemfile_lock_changes).and_return( + <<~GEMFILE_LOCK_DIFF, + - shoulda-matchers (5.3.0) + + shoulda-matchers (6.0.0) + GEMFILE_LOCK_DIFF + ) + + expect(dependency_manager).to receive(:remove_dependency).with( + name: "shoulda-matchers", + version: "5.3.0", + ) + + expect(dependency_manager).to receive(:add_dependency).with( + name: "shoulda-matchers", + version: "6.0.0", + ) + + pull_request.tell_dependency_manager_what_dependabot_is_changing + end end describe "#merge!" do