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.

Example commit updating multiple dependencies: https://github.com/alphagov/content-data-api/pull/1965/commits
Example commit updating single dependency: https://github.com/alphagov/support-api/pull/842/commits

https://trello.com/c/c2KqD9Fu/3328-review-effectiveness-of-version-1-of-the-govuk-dependabot-merger
  • Loading branch information
MuriloDalRi committed Nov 10, 2023
1 parent 901d0c5 commit 3657c50
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 2 deletions.
13 changes: 11 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
end

def gemfile_lock_changes
head_commit.files.find { |file| file.filename == "Gemfile.lock" }.patch
end
Expand All @@ -125,14 +129,19 @@ 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: }] }

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:)
dependency_manager.remove_dependency(name:, version:) if mentioned_dependencies[name]&.fetch(:from_version) == version
end

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

def single_dependency_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

def multiple_dependencies_commit
<<~TEXT
Bump rack, rails and govuk_sidekiq
Bumps [rack](https://github.com/rack/rack), [rails](https://github.com/rails/rails) and [govuk_sidekiq](https://github.com/alphagov/govuk_sidekiq). These dependencies needed to be updated together.
Updates `rack` from 1.0.0 to 1.1.0
- [Release notes](https://github.com/rack/rack/releases)
- [Changelog](https://github.com/rack/rack/blob/main/CHANGELOG.md)
- [Commits](rack/[email protected])
Updates `rails` from 7.0.8 to 7.1.1
- [Release notes](https://github.com/rails/rails/releases)
- [Commits](rails/[email protected])
Updates `govuk_sidekiq` from 5.7.0 to 5.8.0
- [Changelog](https://github.com/alphagov/govuk_sidekiq/blob/main/CHANGELOG.md)
- [Commits](alphagov/[email protected])
---
updated-dependencies:
- dependency-name: rack
dependency-type: direct:development
update-type: version-update:semver-minor
- dependency-name: rails
dependency-type: direct:production
update-type: version-update:semver-minor
- dependency-name: govuk_sidekiq
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 +95,7 @@
author: {
name: "dependabot[bot]",
},
message: single_dependency_commit,
},
author: {
login: "dependabot[bot]",
Expand Down Expand Up @@ -318,6 +374,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(single_dependency_commit)
allow(pull_request).to receive(:gemfile_lock_changes).and_return(
<<~GEMFILE_LOCK_DIFF,
govuk_personalisation (0.13.0)
Expand All @@ -340,6 +397,61 @@ def create_mock_dependency_manager
)
pull_request.tell_dependency_manager_what_dependabot_is_changing
end

it "only looks at the dependencies listed in the commit message" 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(multiple_dependencies_commit)
allow(pull_request).to receive(:gemfile_lock_changes).and_return(
<<~GEMFILE_LOCK_DIFF,
govuk_personalisation (0.13.0)
plek (>= 1.9.0)
rails (>= 6, < 8)
- govuk_publishing_components (35.7.0)
+ govuk_publishing_components (35.8.0)
govuk_app_config
govuk_personalisation (>= 0.7.0)
kramdown
- rack (1.0.0)
+ rack (1.1.0)
- rails (7.0.8)
+ rails (7.1.1)
- govuk_sidekiq (5.7.0)
+ govuk_sidekiq (5.8.0)
GEMFILE_LOCK_DIFF
)

expect(dependency_manager).to receive(:add_dependency).with(
name: "rack",
version: "1.1.0",
)
expect(dependency_manager).to receive(:add_dependency).with(
name: "rails",
version: "7.1.1",
)
expect(dependency_manager).to receive(:add_dependency).with(
name: "govuk_sidekiq",
version: "5.8.0",
)
expect(dependency_manager).to receive(:remove_dependency).with(
name: "rack",
version: "1.0.0",
)
expect(dependency_manager).to receive(:remove_dependency).with(
name: "rails",
version: "7.0.8",
)
expect(dependency_manager).not_to receive(:add_dependency).with(
name: "govuk_publishing_components",
version: "35.8.0",
)
expect(dependency_manager).not_to receive(:remove_dependency).with(
name: "govuk_publishing_components",
version: "35.7.0",
)
pull_request.tell_dependency_manager_what_dependabot_is_changing
end
end

describe "#merge!" do
Expand Down

0 comments on commit 3657c50

Please sign in to comment.