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 prototype for custom test reporter for minitest #3187

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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
96 changes: 96 additions & 0 deletions lib/minitest/reporters/ruby_lsp_reporter.rb
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)
Copy link
Contributor Author

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.

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],
Copy link
Member

Choose a reason for hiding this comment

The 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 Module.const_source_location. Is there some other event, like start_suite that we can remember the file path ahead of time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's a before_suite which takes a Minitest::Reporters::Suite, but that doesn't have file information:

https://github.com/minitest-reporters/minitest-reporters/blob/265ff4b40d5827e84d7e902b808fbee860b61221/lib/minitest/reporters/base_reporter.rb#L3C11-L3C16

We could propose adding this in minitest-reporters.

}
@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
Copy link
Member

Choose a reason for hiding this comment

The 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
=========================================
(eval at /Users/hung-wulo/src/github.com/Shopify/rdoc/test/rdoc/test_rdoc_comment.rb:483)
1
=========================================

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
80 changes: 80 additions & 0 deletions lib/ruby_lsp/test_reporting.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# typed: strict
# frozen_string_literal: true

require "json"

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?
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

@andyw8 andyw8 Feb 18, 2025

Choose a reason for hiding this comment

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

I've seen two variants so far:

  • A Minitest::Assertion failure
  • A Minitest::UnexpectedError, e.g. if something in the test raises

I thought it may be useful to distingish these in the UI, similar to how the progress reporter shows E vs F.

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
6 changes: 6 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
end
Minitest::Reporters.use!(minitest_reporter)

# Temporary for verification
if ENV["RUBY_LSP"]
require "minitest/reporters/ruby_lsp_reporter"
Minitest::Reporters.use!(Minitest::Reporters::RubyLspReporter.new)
end

module Minitest
class Test
extend T::Sig
Expand Down