-
Notifications
You must be signed in to change notification settings - Fork 182
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 prototype for custom test reporter for minitest #3187
base: main
Are you sure you want to change the base?
Changes from 6 commits
30a66c9
db42d90
9ed9e62
41b1834
61c3339
c71eb8b
b7e065f
d11b8fa
5eeb5fb
3a06df3
dff30fc
9cb83bf
d10ceaa
f8a5bc6
8e5ea7e
88ed768
7761d77
81c776c
0e9c212
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
# typed: strict | ||
# frozen_string_literal: true | ||
|
||
require "ruby_lsp/test_reporting" | ||
|
||
module Minitest | ||
module Reporters | ||
class RubyLspReporter < ::Minitest::Reporters::BaseReporter | ||
extend T::Sig | ||
|
||
sig { void } | ||
def initialize | ||
@reporting = T.let(RubyLsp::TestReporter.new, RubyLsp::TestReporter) | ||
super | ||
end | ||
|
||
sig { params(test: Minitest::Test).void } | ||
def before_test(test) | ||
@reporting.before_test( | ||
id: id_from_test(test), | ||
file: file_for_class_name(test), | ||
) | ||
super | ||
end | ||
|
||
sig { params(test: Minitest::Test).void } | ||
def after_test(test) | ||
@reporting.after_test( | ||
id: id_from_test(test), | ||
file: file_for_class_name(test), | ||
) | ||
super | ||
end | ||
|
||
sig { params(test: Minitest::Result).void } | ||
def record(test) | ||
super | ||
|
||
if test.passed? | ||
record_pass(test) | ||
elsif test.skipped? | ||
record_skip(test) | ||
elsif test.failure | ||
record_fail(test) | ||
end | ||
end | ||
|
||
sig { params(result: Minitest::Result).void } | ||
def record_pass(result) | ||
info = { | ||
id: id_from_result(result), | ||
file: result.source_location[0], | ||
} | ||
@reporting.record_pass(**info) | ||
end | ||
|
||
sig { params(result: Minitest::Result).void } | ||
def record_skip(result) | ||
info = { | ||
id: id_from_result(result), | ||
message: result.failure.message, | ||
file: result.source_location[0], | ||
} | ||
@reporting.record_skip(**info) | ||
end | ||
|
||
sig { params(result: Minitest::Result).void } | ||
def record_fail(result) | ||
info = { | ||
id: id_from_result(result), | ||
type: result.failure.class.name, | ||
message: result.failure.message, | ||
file: result.source_location[0], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These record events seem to have access to the file path without having to resort to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a We could propose adding this in |
||
} | ||
@reporting.record_fail(**info) | ||
end | ||
|
||
private | ||
|
||
sig { params(test: Minitest::Test).returns(String) } | ||
def id_from_test(test) | ||
[test.class.name, test.name].join("#") | ||
end | ||
|
||
sig { params(result: Minitest::Result).returns(String) } | ||
def id_from_result(result) | ||
[result.name, result.klass].join("#") | ||
end | ||
|
||
sig { params(test: Minitest::Test).returns(String) } | ||
def file_for_class_name(test) | ||
T.must(Kernel.const_source_location(test.class_name)).first | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a headsup that even in minitest/test-unit it's possible to define test classes dynamically, which make their source location path a bit different, like: class Foo
class_eval <<~RUBY
class FooTest < Test::Unit::TestCase
def test_foo
puts "========================================="
puts Kernel.const_source_location("Foo::FooTest")
puts "========================================="
fail
end
end
RUBY
end
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pushed an update to handle that. Still need to think about what we do in the UI. |
||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
# typed: strict | ||
# frozen_string_literal: true | ||
|
||
require "json" | ||
andyw8 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
module RubyLsp | ||
class TestReporter | ||
extend T::Sig | ||
|
||
sig { params(io: IO).void } | ||
def initialize(io: $stdout) | ||
@io = io | ||
end | ||
|
||
sig { params(id: String, file: String).void } | ||
def before_test(id:, file:) | ||
result = { | ||
event: "before_test", | ||
id: id, | ||
file: file, | ||
} | ||
io.puts result.to_json | ||
end | ||
|
||
sig { params(id: String, file: String).void } | ||
def after_test(id:, file:) | ||
result = { | ||
event: "after_test", | ||
id: id, | ||
file: file, | ||
} | ||
io.puts result.to_json | ||
end | ||
|
||
sig { params(id: String, file: String).void } | ||
def record_pass(id:, file:) | ||
result = { | ||
event: "pass", | ||
id: id, | ||
file: file, | ||
} | ||
io.puts result.to_json | ||
end | ||
|
||
sig do | ||
params( | ||
id: String, | ||
type: T.untyped, # TODO: what type should this be? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is type here? What will we use it for in the extension? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've seen two variants so far:
I thought it may be useful to distingish these in the UI, similar to how the progress reporter shows |
||
message: String, | ||
file: String, | ||
).void | ||
end | ||
def record_fail(id:, type:, message:, file:) | ||
result = { | ||
event: "fail", | ||
type: type, | ||
message: message, | ||
id: id, | ||
file: file, | ||
} | ||
io.puts result.to_json | ||
end | ||
|
||
sig { params(id: String, message: T.nilable(String), file: String).void } | ||
def record_skip(id:, message:, file:) | ||
result = { | ||
event: "skip", | ||
message: message, | ||
id: id, | ||
file: file, | ||
} | ||
io.puts result.to_json | ||
end | ||
|
||
private | ||
|
||
sig { returns(IO) } | ||
attr_reader :io | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is some duplication between the various
record_
methods, but I want to understand what's actually required before cleaning this up. We'll know better once integrated with the extension.