Skip to content

Commit

Permalink
Allow commits by govuk-ci user
Browse files Browse the repository at this point in the history
A new workflow (alphagov/govuk-infrastructure#1342)
adds a commit by the govuk-ci user to Dependabot PRs on gems to bump the version.

These changes allow the auto-merging of those PRs.

https://trello.com/c/R1kkdVjt/3560-re-evaluate-effectiveness-of-gem-auto-release-workflow-3
  • Loading branch information
MuriloDalRi committed Jun 21, 2024
1 parent a7dbb3c commit e10ee2c
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 41 deletions.
19 changes: 7 additions & 12 deletions lib/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ def number
end

def is_auto_mergeable?
if !validate_single_commit
reasons_not_to_merge << "PR contains more than one commit."
elsif !validate_dependabot_commit
reasons_not_to_merge << "PR contains commit not signed by Dependabot."
if !validate_bot_commit
reasons_not_to_merge << "PR contains commit not by Dependabot or govuk-ci."
elsif !validate_files_changed
reasons_not_to_merge << "PR changes files that should not be changed."
elsif !validate_ci_workflow_exists
Expand All @@ -32,15 +30,16 @@ def is_auto_mergeable?
reasons_not_to_merge.count.zero?
end

def validate_single_commit
commits = GitHubClient.instance.pull_request_commits("alphagov/#{@api_response.base.repo.name}", @api_response.number)
commits.count == 1
def validate_bot_commit
commits_json = GitHubClient.instance.pull_request_commits("alphagov/#{@api_response.base.repo.name}", @api_response.number)
commits = JSON.parse(commits_json)
commits.all? { |commit| ["dependabot[bot]", "govuk-ci"].include?(commit["author"]["login"]) }
end

def validate_files_changed
commit = GitHubClient.instance.commit("alphagov/#{@api_response.base.repo.name}", @api_response.head.sha)
files_changed = commit.files.map(&:filename)
allowed_files = ["yarn.lock", "Gemfile.lock", "#{@api_response.base.repo.name}.gemspec"]
allowed_files = ["yarn.lock", "Gemfile.lock", "#{@api_response.base.repo.name}.gemspec", "CHANGELOG.md", "version.rb"]
(files_changed - allowed_files).empty?
end

Expand Down Expand Up @@ -90,10 +89,6 @@ def commit_message
head_commit.message
end

def validate_dependabot_commit
head_commit.verification.verified && head_commit.author.name == "dependabot[bot]"
end

private

def ci_workflow_run_id
Expand Down
71 changes: 42 additions & 29 deletions spec/lib/pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,29 +105,18 @@ def single_dependency_commit
describe "#is_auto_mergeable?" do
def create_pull_request_instance
pr = PullRequest.new(pull_request_api_response)
allow(pr).to receive(:validate_single_commit).and_return(true)
allow(pr).to receive(:validate_bot_commit).and_return(true)
allow(pr).to receive(:validate_files_changed).and_return(true)
allow(pr).to receive(:validate_dependabot_commit).and_return(true)
pr
end

it "should make a call to validate_single_commit" do
it "should make a call to validate_bot_commit" do
pr = create_pull_request_instance
allow(pr).to receive(:validate_single_commit).and_return(false)
expect(pr).to receive(:validate_single_commit)
allow(pr).to receive(:validate_bot_commit).and_return(false)
expect(pr).to receive(:validate_bot_commit)
pr.is_auto_mergeable?
expect(pr.reasons_not_to_merge).to eq([
"PR contains more than one commit.",
])
end

it "should return false if GitHub says the commit is not signed by Dependabot" do
pr = create_pull_request_instance
allow(pr).to receive(:validate_dependabot_commit).and_return(false)
expect(pr).to receive(:validate_dependabot_commit)
pr.is_auto_mergeable?
expect(pr.reasons_not_to_merge).to eq([
"PR contains commit not signed by Dependabot.",
"PR contains commit not by Dependabot or govuk-ci.",
])
end

Expand Down Expand Up @@ -163,33 +152,57 @@ def create_pull_request_instance
end
end

describe "#validate_single_commit" do
let(:commit_response) do
{
sha: "abc123",
commit: {
describe "#validate_bot_commit" do
let(:allowed_bots_commit_response) do
[
{
sha: "abc123",
author: {
login: "dependabot[bot]",
},
},
{
sha: "def456",
author: {
name: "dependabot[bot]",
login: "govuk-ci",
},
},
}
]
end

let(:commit_api_url) { "https://api.github.com/repos/alphagov/#{repo_name}/pulls/1/commits" }

it "return true if PR contains a single commit" do
it "returns true if PR only contains commits by allowed bots" do
stub_request(:get, commit_api_url)
.to_return(status: 200, body: [commit_response])
.to_return(status: 200, body: allowed_bots_commit_response.to_json)

pr = PullRequest.new(pull_request_api_response)
expect(pr.validate_single_commit).to eq(true)
expect(pr.validate_bot_commit).to eq(true)
end

let(:not_allowed_commit_response) do
[
{
sha: "abc123",
author: {
login: "dependabot[bot]",
},
},
{
sha: "def456",
author: {
login: "croissant",
},
},
]
end

it "return false if PR contains more than one commit" do
it "returns false if PR contains commit not by allowed bots" do
stub_request(:get, commit_api_url)
.to_return(status: 200, body: [commit_response, commit_response])
.to_return(status: 200, body: not_allowed_commit_response.to_json)

pr = PullRequest.new(pull_request_api_response)
expect(pr.validate_single_commit).to eq(false)
expect(pr.validate_bot_commit).to eq(false)
end
end

Expand Down

0 comments on commit e10ee2c

Please sign in to comment.