Skip to content

Commit

Permalink
Fix subdependency edge-case
Browse files Browse the repository at this point in the history
A recent PR was auto-merged when it should not have been:
alphagov/content-data-api#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.
  • Loading branch information
ChrisBAshton committed Jan 2, 2024
1 parent 634e362 commit 5101b5e
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 3 deletions.
5 changes: 2 additions & 3 deletions lib/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.]+)\)$/)

Expand Down
45 changes: 45 additions & 0 deletions spec/lib/pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected])
---
updated-dependencies:
- dependency-name: shoulda-matchers
dependency-type: direct:development
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <[email protected]>
TEXT
end

def multiple_dependencies_commit
<<~TEXT
Bump rack, rails and govuk_sidekiq
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5101b5e

Please sign in to comment.