Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error handling for failure while updating Pull request in azure #3493

Merged
merged 4 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions common/lib/dependabot/clients/azure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,11 @@ def update_ref(branch_name, old_commit, new_commit)
}
]

post(source.api_endpoint + source.organization + "/" + source.project +
"/_apis/git/repositories/" + source.unscoped_repo +
"/refs?api-version=5.0", content.to_json)
response = post(source.api_endpoint + source.organization + "/" + source.project +
"/_apis/git/repositories/" + source.unscoped_repo +
"/refs?api-version=5.0", content.to_json)

JSON.parse(response.body).fetch("value").first
end
# rubocop:enable Metrics/ParameterLists

Expand Down
6 changes: 5 additions & 1 deletion common/lib/dependabot/pull_request_updater/azure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
module Dependabot
class PullRequestUpdater
class Azure
class PullRequestUpdateFailed < Dependabot::DependabotError; end

OBJECT_ID_FOR_BRANCH_DELETE = "0000000000000000000000000000000000000000"

attr_reader :source, :files, :base_commit, :old_commit, :credentials,
Expand Down Expand Up @@ -55,9 +57,11 @@ def update_source_branch
# 1) Push the file changes to a newly created temporary branch (from base commit)
new_commit = create_temp_branch
# 2) Update PR source branch to point to the temp branch head commit.
update_branch(source_branch_name, old_source_branch_commit, new_commit)
response = update_branch(source_branch_name, old_source_branch_commit, new_commit)
# 3) Delete temp branch
update_branch(temp_branch_name, new_commit, OBJECT_ID_FOR_BRANCH_DELETE)

raise PullRequestUpdateFailed, response.fetch("customMessage", nil) unless response.fetch("success", false)
end

def pull_request
Expand Down
2 changes: 1 addition & 1 deletion common/spec/dependabot/clients/azure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@
it "sends update branch request with old and new commit id" do
stub_request(:post, update_ref_url).
with(basic_auth: [username, password]).
to_return(status: 200)
to_return(status: 200, body: fixture("azure", "update_ref.json"))

update_ref

Expand Down
114 changes: 64 additions & 50 deletions common/spec/dependabot/pull_request_updater/azure_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,57 +113,71 @@
end
end

it "updates source branch head commit in AzureDevOps" do
stub_request(:get, source_branch_commits_url).
to_return(status: 200,
body: fixture("azure", "commits.json"),
headers: json_header)
stub_request(:get, repo_contents_tree_url).
to_return(status: 200,
body: fixture("azure", "repo_contents_treeroot.json"),
headers: json_header)
stub_request(:get, repo_contents_url).
to_return(status: 200,
body: fixture("azure", "repo_contents.json"),
headers: json_header)
stub_request(:post, create_commit_url).
with(body: {
refUpdates: [
{ name: "refs/heads/#{temp_branch}", oldObjectId: base_commit }
],
commits: [
{
comment: commit_message,
changes: files.map do |file|
{
changeType: "edit",
item: { path: file.path },
newContent: {
content: Base64.encode64(file.content),
contentType: "base64encoded"
context "tries updating source branch head commit in AzureDevOps" do
before do
stub_request(:get, source_branch_commits_url).
to_return(status: 200,
body: fixture("azure", "commits.json"),
headers: json_header)
stub_request(:get, repo_contents_tree_url).
to_return(status: 200,
body: fixture("azure", "repo_contents_treeroot.json"),
headers: json_header)
stub_request(:get, repo_contents_url).
to_return(status: 200,
body: fixture("azure", "repo_contents.json"),
headers: json_header)
stub_request(:post, create_commit_url).
with(body: {
refUpdates: [
{ name: "refs/heads/#{temp_branch}", oldObjectId: base_commit }
],
commits: [
{
comment: commit_message,
changes: files.map do |file|
{
changeType: "edit",
item: { path: file.path },
newContent: {
content: Base64.encode64(file.content),
contentType: "base64encoded"
}
}
}
end
}
]
}).
to_return(status: 201,
body: fixture("azure", "create_new_branch.json"),
headers: json_header)
stub_request(:post, branch_update_url).
to_return(status: 201)

allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
updater.update

expect(WebMock).
to(
have_requested(:post, branch_update_url).
with(body: [
{ name: "refs/heads/#{source_branch}", oldObjectId: source_branch_old_commit,
newObjectId: source_branch_new_commit }
].to_json)
)
end
}
]
}).
to_return(status: 201,
body: fixture("azure", "create_new_branch.json"),
headers: json_header)

allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
end

it "sends request to AzureDevOps to update source branch head commit" do
stub_request(:post, branch_update_url).
to_return(status: 201, body: fixture("azure", "update_ref.json"))

allow(updater).to receive(:temp_branch_name).and_return(temp_branch)
updater.update

expect(WebMock).
to(
have_requested(:post, branch_update_url).
with(body: [
{ name: "refs/heads/#{source_branch}", oldObjectId: source_branch_old_commit,
newObjectId: source_branch_new_commit }
].to_json)
)
end

it "raises a helpful error when source branch update is unsuccessful" do
stub_request(:post, branch_update_url).
to_return(status: 200, body: fixture("azure", "update_ref_failed.json"))

expect { updater.update }.to raise_error(Dependabot::PullRequestUpdater::Azure::PullRequestUpdateFailed)
end
end
end
end
14 changes: 14 additions & 0 deletions common/spec/fixtures/azure/update_ref.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"value": [
{
"repositoryId": "d3d1760b-311c-4175-a726-20dfc6a7f885",
"name": "refs/heads/vsts-api-sample/answer-woman-flame",
"oldObjectId": "ggf0dcb632e11e8e71f433956183349746fec562",
"newObjectId": "ffe9cba521f00d7f60e322845072238635edb451",
"isLocked": false,
"updateStatus": "succeeded",
"success": true
}
],
"count": 1
}
14 changes: 14 additions & 0 deletions common/spec/fixtures/azure/update_ref_failed.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"value": [
{
"repositoryId": "d3d1760b-311c-4175-a726-20dfc6a7f885",
"name": "refs/heads/vsts-api-sample/answer-woman-flame",
"oldObjectId": "ggf0dcb632e11e8e71f433956183349746fec562",
"customMessage": "The user does not have force push permissions",
"isLocked": false,
"updateStatus": "failed",
"success": false
}
],
"count": 1
}