Skip to content

Commit

Permalink
Check only for changes mentioned in the commit message
Browse files Browse the repository at this point in the history
Only check the gemfile.lock for changes that have been mentioned in the commit message.
This is because we want to ignore sub-dependencies.

https://trello.com/c/c2KqD9Fu/3328-review-effectiveness-of-version-1-of-the-govuk-dependabot-merger
  • Loading branch information
MuriloDalRi committed Nov 6, 2023
1 parent 901d0c5 commit 7085f4d
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
26 changes: 24 additions & 2 deletions lib/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ def head_commit
@head_commit ||= GitHubClient.instance.commit("alphagov/#{@api_response.base.repo.name}", @api_response.head.sha)
end

def commit_message
head_commit.commit.message if head_commit
end

def gemfile_lock_changes
head_commit.files.find { |file| file.filename == "Gemfile.lock" }.patch
end
Expand All @@ -125,14 +129,32 @@ def tell_dependency_manager_what_dependencies_are_allowed
end

def tell_dependency_manager_what_dependabot_is_changing
dependency_updates = commit_message.scan(/Updates `(\w+)` from (\d+\.\d+\.\d+) to (\d+\.\d+\.\d)/)

# Commit messages can have different formats
if dependency_updates.empty?
dependency_updates = commit_message.scan(/Bump (?:\[.*?\]\(.+?\) )?(\w+) from (\d+\.\d+\.\d+) to (\d+\.\d+\.\d)/)
end

mentioned_dependencies = {}

dependency_updates.each do |name, from_version, to_version|
mentioned_dependencies[name] = [from_version, to_version]
end

lines_removed = gemfile_lock_changes.scan(/^-\s+([a-z\-_]+) \(([0-9.]+)\)$/)
lines_added = gemfile_lock_changes.scan(/^\+\s+([a-z\-_]+) \(([0-9.]+)\)$/)

lines_removed.each do |name, version|
dependency_manager.remove_dependency(name:, version:)
if mentioned_dependencies.key?(name) && mentioned_dependencies[name][0] == version
dependency_manager.remove_dependency(name:, version:)
end
end

lines_added.each do |name, version|
dependency_manager.add_dependency(name:, version:)
if mentioned_dependencies.key?(name) && mentioned_dependencies[name][1] == version
dependency_manager.add_dependency(name:, version:)
end
end
end
end
21 changes: 21 additions & 0 deletions spec/lib/pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,25 @@
RSpec.describe PullRequest do
before { set_up_mock_token }

def fake_commit
<<~TEXT
Bump govuk_publishing_components from 35.7.0 to 35.8.0
Bumps [govuk_publishing_components](https://github.com/alphagov/govuk_publishing_components) from 35.7.0 to 35.8.0.
- [Changelog](https://github.com/alphagov/govuk_publishing_components/blob/main/CHANGELOG.md)
- [Commits](alphagov/[email protected])
---
updated-dependencies:
- dependency-name: govuk_publishing_components
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <[email protected]>
TEXT
end

let(:repo_name) { "foo" }
let(:sha) { "ee241dea8da11aff8e575941c138a7f34ddb1a51" }
let(:pull_request_api_response) do
Expand Down Expand Up @@ -40,6 +59,7 @@
author: {
name: "dependabot[bot]",
},
message: fake_commit,
},
author: {
login: "dependabot[bot]",
Expand Down Expand Up @@ -318,6 +338,7 @@ def create_mock_dependency_manager
dependency_manager = double("DependencyManager")
api_response = "foo"
pull_request = PullRequest.new(api_response, dependency_manager)
allow(pull_request).to receive(:commit_message).and_return(fake_commit)
allow(pull_request).to receive(:gemfile_lock_changes).and_return(
<<~GEMFILE_LOCK_DIFF,
govuk_personalisation (0.13.0)
Expand Down

0 comments on commit 7085f4d

Please sign in to comment.