Skip to content

Commit

Permalink
Limit the size of GitHub review body comment
Browse files Browse the repository at this point in the history
GitHub limits the review body (main comment) to be 65,536 characters or
less. Otherwise, an error gets thrown.
This change will truncate our errors to fit within that limit, while
still trying to show the error.
  • Loading branch information
Greg Lazarev committed May 29, 2017
1 parent 1d6b639 commit 3fa5782
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 64 deletions.
18 changes: 1 addition & 17 deletions app/models/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def make_comments(violations, errors)
repo_name,
number,
comments,
format_errors(errors),
ReviewBody.new(errors).to_s,
)
end

Expand Down Expand Up @@ -67,22 +67,6 @@ def build_comment(violation)
}
end

def format_errors(errors)
if errors.any?
error_details = errors.map { |error| build_error_details(error) }
["Some files could not be reviewed due to errors:"].
concat(error_details).join
else
""
end
end

def build_error_details(error)
summary = error.lines.first.strip.truncate(80)
details = error.lines.map(&:rstrip).join(COMMENT_LINE_DELIMITER)
"<details><summary>#{summary}</summary><pre>#{details}</pre></details>"
end

def user_github
@user_github ||= GithubApi.new(token)
end
Expand Down
55 changes: 55 additions & 0 deletions app/models/review_body.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# frozen_string_literal: true
class ReviewBody
MAX_BODY_LENGTH = 65536
SUMMARY_LENGTH = 80
LINE_DELIMITER = "<br>"
HEADER = "Some files could not be reviewed due to errors:"
DETAILS_FORMAT = "<details><summary>%s</summary><pre>%s</pre></details>"

def initialize(errors)
@errors = errors
end

def to_s
if errors.any?
error_details = errors.map { |error| build_error_details(error) }
[HEADER].concat(error_details).join
else
""
end
end

private

attr_reader :errors

def build_error_details(error)
summary = error_summary(error)
details = error[0...(allowed_error_length - summary.length)].
lines.
map(&:rstrip).
join(LINE_DELIMITER)

sprintf(DETAILS_FORMAT, summary, details)
end

def error_summary(error)
error.lines.first.strip.truncate(SUMMARY_LENGTH)
end

def allowed_error_length
(MAX_BODY_LENGTH - formatting_characters_size) / errors.size
end

def formatting_characters_size
HEADER.length + all_detail_formats_length + all_lines_delimiters_length
end

def all_detail_formats_length
DETAILS_FORMAT.size * errors.size
end

def all_lines_delimiters_length
(errors.flat_map(&:lines).size - errors.size) * LINE_DELIMITER.length
end
end
68 changes: 21 additions & 47 deletions spec/models/pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,57 +60,31 @@
end

describe "#make_comments" do
context "when no errors in file reviews exist" do
it "posts a review with comments to GitHub as the Hound user" do
payload = payload_stub
github = instance_double("GithubApi", create_pull_request_review: nil)
pull_request = pull_request_stub(github, payload)
violation = violation_stub

pull_request.make_comments([violation], [])

expect(github).to have_received(:create_pull_request_review).with(
"org/repo",
payload.pull_request_number,
[
{
path: violation.filename,
position: violation.patch_position,
body: violation.messages.join,
},
],
""
)
end
end

context "when file reviews contain errors" do
it "posts a review with comments to GitHub as the Hound user" do
payload = payload_stub
github = instance_double("GithubApi", create_pull_request_review: nil)
pull_request = pull_request_stub(github, payload)
violation = violation_stub
review_errors = ["invalid config", "foo\n bar"]

pull_request.make_comments([violation], review_errors)

expect(github).to have_received(:create_pull_request_review).with(
"org/repo",
payload.pull_request_number,
[
{
path: violation.filename,
position: violation.patch_position,
body: violation.messages.join,
},
],
"Some files could not be reviewed due to errors:" \
it "posts a review with comments to GitHub as the Hound user" do
payload = payload_stub
github = instance_double("GithubApi", create_pull_request_review: nil)
pull_request = pull_request_stub(github, payload)
violation = violation_stub
review_errors = ["invalid config", "foo\n bar"]

pull_request.make_comments([violation], review_errors)

expect(github).to have_received(:create_pull_request_review).with(
"org/repo",
payload.pull_request_number,
[
{
path: violation.filename,
position: violation.patch_position,
body: violation.messages.join,
},
],
"Some files could not be reviewed due to errors:" \
"<details><summary>invalid config</summary>" \
"<pre>invalid config</pre></details>" \
"<details><summary>foo</summary>" \
"<pre>foo<br> bar</pre></details>",
)
end
)
end
end

Expand Down
53 changes: 53 additions & 0 deletions spec/models/review_body_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
require "app/models/review_body"

RSpec.describe ReviewBody do
describe "#to_s" do
context "when no errors are passed" do
it "returns empty string" do
review_body = ReviewBody.new([])

result = review_body.to_s

expect(result).to eq ""
end
end

context "with errors" do
it "returns formatted errors" do
review_body = ReviewBody.new(["invalid config", "foo\n bar"])

result = review_body.to_s

expect(result).to eq(
"Some files could not be reviewed due to errors:" \
"<details><summary>invalid config</summary>" \
"<pre>invalid config</pre></details>" \
"<details><summary>foo</summary>" \
"<pre>foo<br> bar</pre></details>",
)
end

context "when errors are too long" do
it "truncates the errors" do
stub_const("ReviewBody::MAX_BODY_LENGTH", 200)
review_body = ReviewBody.new(
[
"invalid config",
"rule is unknown\n MyRule",
],
)

result = review_body.to_s

expect(result).to eq(
"Some files could not be reviewed due to errors:" \
"<details><summary>invalid config</summary>" \
"<pre>invalid</pre></details>" \
"<details><summary>rule is unknown</summary>" \
"<pre>rule i</pre></details>",
)
end
end
end
end
end

0 comments on commit 3fa5782

Please sign in to comment.