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 support for summaries in GitHubActionsFormatter #531

Merged
merged 2 commits into from
Dec 11, 2024
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
79 changes: 73 additions & 6 deletions spec/ameba/formatter/github_actions_formatter_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,22 @@ require "../../spec_helper"

module Ameba::Formatter
describe GitHubActionsFormatter do
output = IO::Memory.new
subject = GitHubActionsFormatter.new(output)

before_each do
output.clear
end

describe "#source_finished" do
it "writes valid source" do
output = IO::Memory.new
subject = GitHubActionsFormatter.new(output)

source = Source.new "", "/path/to/file.cr"

subject.source_finished(source)
output.to_s.should be_empty
end

it "writes invalid source" do
output = IO::Memory.new
subject = GitHubActionsFormatter.new(output)

source = Source.new "", "/path/to/file.cr"
location = Crystal::Location.new("/path/to/file.cr", 1, 2)

Expand All @@ -26,5 +27,71 @@ module Ameba::Formatter
output.to_s.should eq("::notice file=/path/to/file.cr,line=1,col=2,endLine=1,endColumn=2,title=Ameba/DummyRule::message%0A2nd line\n")
end
end

describe "#finished" do
it "doesn't do anything if 'GITHUB_STEP_SUMMARY' ENV var is not set" do
subject.finished [Source.new ""]
output.to_s.should be_empty
end

it "writes a Markdown summary to a filename given in 'GITHUB_STEP_SUMMARY' ENV var" do
prev_summary = ENV["GITHUB_STEP_SUMMARY"]?
ENV["GITHUB_STEP_SUMMARY"] = summary_filename = File.tempname
begin
sources = [Source.new ""]

subject.started(sources)
subject.finished(sources)

File.exists?(summary_filename).should be_true

summary = File.read(summary_filename)
summary.should contain "## Ameba Results :green_heart:"
summary.should contain "Finished in"
summary.should contain "**1** sources inspected, **0** failures."
summary.should contain "> Ameba version: **#{Ameba::VERSION}**"
ensure
ENV["GITHUB_STEP_SUMMARY"] = prev_summary
File.delete(summary_filename) rescue nil
end
end

context "when issues found" do
it "writes each issue" do
prev_summary = ENV["GITHUB_STEP_SUMMARY"]?
ENV["GITHUB_STEP_SUMMARY"] = summary_filename = File.tempname

repo = ENV["GITHUB_REPOSITORY"]?
sha = ENV["GITHUB_SHA"]?
begin
source = Source.new("", "src/source.cr")
source.add_issue(DummyRule.new, {1, 1}, {2, 1}, "DummyRuleError")
source.add_issue(DummyRule.new, {1, 1}, "DummyRuleError 2")
source.add_issue(NamedRule.new, {1, 2}, "NamedRuleError", status: :disabled)

subject.finished([source])

File.exists?(summary_filename).should be_true

summary = File.read(summary_filename)
summary.should contain "## Ameba Results :bug:"
summary.should contain "### Issues found:"
summary.should contain "#### `src/source.cr` (**2** issues)"
if repo && sha
summary.should contain "| [1-2](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1-L2) | Convention | Ameba/DummyRule | DummyRuleError |"
summary.should contain "| [1](https://github.com/#{repo}/blob/#{sha}/src/source.cr#L1) | Convention | Ameba/DummyRule | DummyRuleError 2 |"
else
summary.should contain "| 1-2 | Convention | Ameba/DummyRule | DummyRuleError |"
summary.should contain "| 1 | Convention | Ameba/DummyRule | DummyRuleError 2 |"
end
summary.should_not contain "NamedRuleError"
summary.should contain "**1** sources inspected, **2** failures."
ensure
ENV["GITHUB_STEP_SUMMARY"] = prev_summary
File.delete(summary_filename) rescue nil
end
end
end
end
end
end
24 changes: 1 addition & 23 deletions src/ameba/formatter/dot_formatter.cr
Original file line number Diff line number Diff line change
Expand Up @@ -77,34 +77,12 @@ module Ameba::Formatter
"Finished in #{to_human(finished - started)}".colorize(:default)
end

private def to_human(span : Time::Span)
total_milliseconds = span.total_milliseconds
if total_milliseconds < 1
return "#{(span.total_milliseconds * 1_000).round.to_i} microseconds"
end

total_seconds = span.total_seconds
if total_seconds < 1
return "#{span.total_milliseconds.round(2)} milliseconds"
end

if total_seconds < 60
return "#{total_seconds.round(2)} seconds"
end

minutes = span.minutes
seconds = span.seconds

"#{minutes}:#{seconds < 10 ? "0" : ""}#{seconds} minutes"
end

private def final_message(sources, failed_sources)
total = sources.size
failures = failed_sources.sum(&.issues.count(&.enabled?))
color = failures == 0 ? :green : :red
s = failures != 1 ? "s" : ""

"#{total} inspected, #{failures} failure#{s}".colorize(color)
"#{total} inspected, #{failures} #{pluralize(failures, "failure")}".colorize(color)
end
end
end
112 changes: 112 additions & 0 deletions src/ameba/formatter/github_actions_formatter.cr
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
require "./util"

module Ameba::Formatter
# A formatter that outputs issues in a GitHub Actions compatible format.
#
# See [GitHub Actions documentation](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/workflow-commands-for-github-actions) for details.
class GitHubActionsFormatter < BaseFormatter
include Util

@started_at : Time::Span?
@mutex = Mutex.new

# Reports a message when inspection is started.
def started(sources) : Nil
@started_at = Time.monotonic
end

# Reports a result of the inspection of a corresponding source.
def source_finished(source : Source) : Nil
source.issues.each do |issue|
Expand Down Expand Up @@ -37,6 +47,108 @@ module Ameba::Formatter
end
end

# Reports a message when inspection is finished.
def finished(sources) : Nil
return unless step_summary_file = ENV["GITHUB_STEP_SUMMARY"]?

if started_at = @started_at
time_elapsed = Time.monotonic - started_at
end

File.write(step_summary_file, summary(sources, time_elapsed))
end

private def summary(sources, time_elapsed)
failed_sources = sources.reject(&.valid?)
total = sources.size
failures = failed_sources.sum(&.issues.count(&.enabled?))

String.build do |output|
output << "## Ameba Results %s\n\n" % {
failures == 0 ? ":green_heart:" : ":bug:",
}

if failures.positive?
output << "### Issues found:\n\n"

failed_sources.each do |source|
issue_count = source.issues.count(&.enabled?)

if issue_count.positive?
output << "#### `%s` (**%d** %s)\n\n" % {
source.path,
issue_count,
pluralize(issue_count, "issue"),
}

output.puts "| Line | Severity | Name | Message |"
output.puts "| ---- | -------- | ---- | ------- |"

source.issues.each do |issue|
next if issue.disabled?

output.puts "| %s | %s | %s | %s |" % {
issue_location_value(issue) || "-",
issue.rule.severity,
issue.rule.name,
Copy link
Contributor

@straight-shoota straight-shoota Dec 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Maybe the rule name could include a link to the documentation? This could be a future enhancement, of course.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about it, although atm we could only link directly to the ameba repo, since we don't have versioned docs (yet).

See related discussion in vscode-crystal-ameba repo: crystal-ameba/vscode-crystal-ameba#146 (comment)

issue.message,
}
end
output << "\n"
end
end
output << "\n"
end

if time_elapsed
output.puts "Finished in %s." % to_human(time_elapsed)
end
output.puts "**%d** sources inspected, **%d** %s." % {
total,
failures,
pluralize(failures, "failure"),
}
output.puts
output.puts "> Ameba version: **%s**" % Ameba::VERSION
end
end

private BLOB_URL = begin
repo = ENV["GITHUB_REPOSITORY"]?
sha = ENV["GITHUB_SHA"]?

if repo && sha
"https://github.com/#{repo}/blob/#{sha}"
end
end

private def issue_location_value(issue)
location, end_location =
issue.location, issue.end_location

return unless location

line_selector =
if end_location && location.line_number != end_location.line_number
"#{location.line_number}-#{end_location.line_number}"
else
"#{location.line_number}"
end

if BLOB_URL
location_url = "[%s](%s/%s#%s)" % {
line_selector,
BLOB_URL,
location.filename,
line_selector
.split('-')
.join('-') { |i| "L#{i}" },
}
end

location_url || line_selector
end

private def command_name(severity : Severity) : String
case severity
in .error? then "error"
Expand Down
25 changes: 25 additions & 0 deletions src/ameba/formatter/util.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,31 @@ module Ameba::Formatter
module Util
extend self

def pluralize(count : Int, singular : String, plural = "#{singular}s")
count == 1 ? singular : plural
end

def to_human(span : Time::Span)
total_milliseconds = span.total_milliseconds
if total_milliseconds < 1
return "#{(span.total_milliseconds * 1_000).round.to_i} microseconds"
end

total_seconds = span.total_seconds
if total_seconds < 1
return "#{span.total_milliseconds.round(2)} milliseconds"
end

if total_seconds < 60
return "#{total_seconds.round(2)} seconds"
end

minutes = span.minutes
seconds = span.seconds

"#{minutes}:#{seconds < 10 ? "0" : ""}#{seconds} minutes"
end

def deansify(message : String?) : String?
message.try &.gsub(/\x1b[^m]*m/, "").presence
end
Expand Down
Loading