Skip to content

Commit 1c135c1

Browse files
jcolemanMorriar
jcoleman
and
Morriar
committed
Code review feedback
Co-authored-by: Alexandre Terrasa <[email protected]>
1 parent 0e8b24b commit 1c135c1

File tree

3 files changed

+153
-153
lines changed

3 files changed

+153
-153
lines changed

‎lib/spoom/cli/srb/tc.rb

+2-54
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
# typed: true
22
# frozen_string_literal: true
33

4-
require "rexml/document"
5-
64
module Spoom
75
module Cli
86
module Srb
@@ -60,7 +58,7 @@ def tc(*paths_to_select)
6058
if result.status
6159
say_error(result.err, status: nil, nl: false)
6260
if junit_output_path
63-
write_errors_to_xml([], path: junit_output_path)
61+
Spoom::Sorbet::Errors.to_junit_xml([], path: junit_output_path)
6462
end
6563
exit(0)
6664
end
@@ -102,7 +100,7 @@ def tc(*paths_to_select)
102100
end
103101

104102
if junit_output_path
105-
write_errors_to_xml(errors, path: junit_output_path)
103+
Spoom::Sorbet::Errors.to_junit_xml(errors, path: junit_output_path)
106104
end
107105

108106
if count
@@ -154,56 +152,6 @@ def colorize_message(message)
154152
end
155153
word.string
156154
end
157-
158-
def write_errors_to_xml(errors, path:)
159-
doc = REXML::Document.new
160-
doc << REXML::XMLDecl.new
161-
testsuite_element = doc.add_element("testsuite")
162-
testsuite_element.add_attributes(
163-
"name" => "Sorbet",
164-
"failures" => errors.size,
165-
)
166-
167-
if errors.empty?
168-
# Avoid creating an empty report when there are no errors so that
169-
# reporting tools know that the type checking ran successfully.
170-
testcase_element = testsuite_element.add_element("testcase")
171-
testcase_element.add_attributes(
172-
"name" => "Typecheck",
173-
"tests" => 1,
174-
)
175-
else
176-
errors.each do |error|
177-
testcase_element = testsuite_element.add_element("testcase")
178-
# Unlike traditional test suites, we can't report all tests
179-
# regardless of outcome; we only have errors to report. As a
180-
# result we reinterpret the definitions of the test properties
181-
# bit: the error message becomes the test name and the full error
182-
# info gets plugged into the failure body along with file/line
183-
# information (displayed in Jenkins as the "Stacktrace" for the
184-
# error).
185-
testcase_element.add_attributes(
186-
"name" => error.message,
187-
"file" => error.file,
188-
"line" => error.line,
189-
)
190-
failure_element = testcase_element.add_element("failure")
191-
failure_element.add_attributes(
192-
"type" => error.code,
193-
)
194-
explanation_lines = [
195-
"In file #{error.file}:\n",
196-
] + error.more
197-
explanation_text = explanation_lines.join("").chomp
198-
# Use CDATA so that parsers know the whitespace is significant.
199-
failure_element.add(REXML::CData.new(explanation_text))
200-
end
201-
end
202-
203-
xml_buffer = StringIO.new
204-
doc.write(output: xml_buffer, indent: 2)
205-
File.write(path, xml_buffer.string)
206-
end
207155
end
208156
end
209157
end

‎lib/spoom/sorbet/errors.rb

+60
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# typed: strict
22
# frozen_string_literal: true
33

4+
require "rexml/document"
5+
46
module Spoom
57
module Sorbet
68
module Errors
@@ -13,6 +15,36 @@ class << self
1315
def sort_errors_by_code(errors)
1416
errors.sort_by { |e| [e.code, e.file, e.line, e.message] }
1517
end
18+
19+
def to_junit_xml(errors, path:)
20+
testsuite_element = REXML::Element.new("testsuite")
21+
testsuite_element.add_attributes(
22+
"name" => "Sorbet",
23+
"failures" => errors.size,
24+
)
25+
26+
if errors.empty?
27+
# Avoid creating an empty report when there are no errors so that
28+
# reporting tools know that the type checking ran successfully.
29+
testcase_element = testsuite_element.add_element("testcase")
30+
testcase_element.add_attributes(
31+
"name" => "Typecheck",
32+
"tests" => 1,
33+
)
34+
else
35+
errors.each do |error|
36+
testsuite_element.add_element(error.to_junit_xml_element)
37+
end
38+
end
39+
40+
doc = REXML::Document.new
41+
doc << REXML::XMLDecl.new
42+
doc.add_element(testsuite_element)
43+
44+
file = File.open(path, "w")
45+
doc.write(output: file, indent: 2)
46+
file.close
47+
end
1648
end
1749
# Parse errors from Sorbet output
1850
class Parser
@@ -163,6 +195,34 @@ def <=>(other)
163195
def to_s
164196
"#{file}:#{line}: #{message} (#{code})"
165197
end
198+
199+
def to_junit_xml_element
200+
testcase_element = REXML::Element.new("testcase")
201+
# Unlike traditional test suites, we can't report all tests
202+
# regardless of outcome; we only have errors to report. As a
203+
# result we reinterpret the definitions of the test properties
204+
# bit: the error message becomes the test name and the full error
205+
# info gets plugged into the failure body along with file/line
206+
# information (displayed in Jenkins as the "Stacktrace" for the
207+
# error).
208+
testcase_element.add_attributes(
209+
"name" => message,
210+
"file" => file,
211+
"line" => line,
212+
)
213+
failure_element = testcase_element.add_element("failure")
214+
failure_element.add_attributes(
215+
"type" => code,
216+
)
217+
explanation_text = [
218+
"In file #{file}:\n",
219+
*more
220+
].join.chomp
221+
# Use CDATA so that parsers know the whitespace is significant.
222+
failure_element.add(REXML::CData.new(explanation_text))
223+
224+
testcase_element
225+
end
166226
end
167227
end
168228
end

‎test/spoom/cli/srb/tc_test.rb

+91-99
Original file line numberDiff line numberDiff line change
@@ -273,108 +273,100 @@ def test_display_errors_with_path_option
273273

274274
def test_output_no_errors_to_junit_xml
275275
@project.write_sorbet_config!("file.rb")
276-
Tempfile.create("sorbet_junit") do |tempfile|
277-
result = @project.spoom("srb tc --junit_output_path=#{tempfile.path}")
278-
expected_doc = <<~XML.chomp
279-
<?xml version='1.0'?>
280-
<testsuite name='Sorbet' failures='0'>
281-
<testcase name='Typecheck' tests='1'/>
282-
</testsuite>
283-
XML
284-
tempfile.rewind
285-
actual_doc = tempfile.read
286-
assert_equal(expected_doc, actual_doc)
287-
assert(result.status)
288-
end
276+
result = @project.spoom("srb tc --junit_output_path=junit.xml")
277+
expected_doc = <<~XML.chomp
278+
<?xml version='1.0'?>
279+
<testsuite name='Sorbet' failures='0'>
280+
<testcase name='Typecheck' tests='1'/>
281+
</testsuite>
282+
XML
283+
assert_equal(expected_doc, @project.read("junit.xml"))
284+
assert(result.status)
289285
end
290286

291287
def test_output_errors_to_junit_xml
292-
Tempfile.create("sorbet_junit") do |tempfile|
293-
expected_doc = <<~XML.chomp
294-
<?xml version='1.0'?>
295-
<testsuite name='Sorbet' failures='7'>
296-
<testcase name='Unable to resolve constant `Bar`' file='errors/errors.rb' line='5'>
297-
<failure type='5002'>
298-
<![CDATA[In file errors/errors.rb:
299-
5 | sig { params(bar: Bar).returns(C) }
300-
^^^]]>
301-
</failure>
302-
</testcase>
303-
<testcase name='Unable to resolve constant `C`' file='errors/errors.rb' line='5'>
304-
<failure type='5002'>
305-
<![CDATA[In file errors/errors.rb:
306-
5 | sig { params(bar: Bar).returns(C) }
307-
^
308-
Did you mean `RDoc::Parser::C`? Use `-a` to autocorrect
309-
errors/errors.rb:5: Replace with `RDoc::Parser::C`
310-
5 | sig { params(bar: Bar).returns(C) }
311-
^
312-
https://github.com/sorbet/sorbet/tree/dad16b7fe5bf8ebdec30bac0d9c1cb0e66f582cf/rbi/stdlib/rdoc.rbi#L6768: `RDoc::Parser::C` defined here
313-
6768 |class RDoc::Parser::C < ::RDoc::Parser
314-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^]]>
315-
</failure>
316-
</testcase>
317-
<testcase name='Method `params` does not exist on `T.class_of(Foo)`' file='errors/errors.rb' line='5'>
318-
<failure type='7003'>
319-
<![CDATA[In file errors/errors.rb:
320-
5 | sig { params(bar: Bar).returns(C) }
321-
^^^^^^]]>
322-
</failure>
323-
</testcase>
324-
<testcase name='Method `sig` does not exist on `T.class_of(Foo)`' file='errors/errors.rb' line='5'>
325-
<failure type='7003'>
326-
<![CDATA[In file errors/errors.rb:
327-
5 | sig { params(bar: Bar).returns(C) }
328-
^^^
329-
Autocorrect: Use `-a` to autocorrect
330-
errors/errors.rb:5: Insert `extend T::Sig`
331-
5 | sig { params(bar: Bar).returns(C) }
332-
^]]>
333-
</failure>
334-
</testcase>
335-
<testcase name='Wrong number of arguments for constructor. Expected: `0`, got: `1`' file='errors/errors.rb' line='10'>
336-
<failure type='7004'>
337-
<![CDATA[In file errors/errors.rb:
338-
10 |b = Foo.new(42)
339-
^^
340-
https://github.com/sorbet/sorbet/tree/dad16b7fe5bf8ebdec30bac0d9c1cb0e66f582cf/rbi/core/basic_object.rbi#L228: `initialize` defined here
341-
228 | def initialize(); end
342-
^^^^^^^^^^^^^^^^
343-
Autocorrect: Use `-a` to autocorrect
344-
errors/errors.rb:10: Delete
345-
10 |b = Foo.new(42)
346-
^^]]>
347-
</failure>
348-
</testcase>
349-
<testcase name='Method `c` does not exist on `T.class_of(&lt;root&gt;)`' file='errors/errors.rb' line='11'>
350-
<failure type='7003'>
351-
<![CDATA[In file errors/errors.rb:
352-
11 |b.foo(b, c)
353-
^]]>
354-
</failure>
355-
</testcase>
356-
<testcase name='Too many arguments provided for method `Foo#foo`. Expected: `1`, got: `2`' file='errors/errors.rb' line='11'>
357-
<failure type='7004'>
358-
<![CDATA[In file errors/errors.rb:
359-
11 |b.foo(b, c)
360-
^
361-
errors/errors.rb:6: `foo` defined here
362-
6 | def foo(bar)
363-
^^^^^^^^^^^^
364-
Autocorrect: Use `-a` to autocorrect
365-
errors/errors.rb:11: Delete
366-
11 |b.foo(b, c)
367-
^^^]]>
368-
</failure>
369-
</testcase>
370-
</testsuite>
371-
XML
372-
result = @project.spoom("srb tc --junit_output_path=#{tempfile.path}")
373-
tempfile.rewind
374-
actual_doc = tempfile.read
375-
assert_equal(expected_doc, actual_doc)
376-
refute(result.status)
377-
end
288+
result = @project.spoom("srb tc --junit_output_path=junit.xml")
289+
expected_doc = <<~XML.chomp
290+
<?xml version='1.0'?>
291+
<testsuite name='Sorbet' failures='7'>
292+
<testcase name='Unable to resolve constant `Bar`' file='errors/errors.rb' line='5'>
293+
<failure type='5002'>
294+
<![CDATA[In file errors/errors.rb:
295+
5 | sig { params(bar: Bar).returns(C) }
296+
^^^]]>
297+
</failure>
298+
</testcase>
299+
<testcase name='Unable to resolve constant `C`' file='errors/errors.rb' line='5'>
300+
<failure type='5002'>
301+
<![CDATA[In file errors/errors.rb:
302+
5 | sig { params(bar: Bar).returns(C) }
303+
^
304+
Did you mean `RDoc::Parser::C`? Use `-a` to autocorrect
305+
errors/errors.rb:5: Replace with `RDoc::Parser::C`
306+
5 | sig { params(bar: Bar).returns(C) }
307+
^
308+
https://github.com/sorbet/sorbet/tree/dad16b7fe5bf8ebdec30bac0d9c1cb0e66f582cf/rbi/stdlib/rdoc.rbi#L6768: `RDoc::Parser::C` defined here
309+
6768 |class RDoc::Parser::C < ::RDoc::Parser
310+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^]]>
311+
</failure>
312+
</testcase>
313+
<testcase name='Method `params` does not exist on `T.class_of(Foo)`' file='errors/errors.rb' line='5'>
314+
<failure type='7003'>
315+
<![CDATA[In file errors/errors.rb:
316+
5 | sig { params(bar: Bar).returns(C) }
317+
^^^^^^]]>
318+
</failure>
319+
</testcase>
320+
<testcase name='Method `sig` does not exist on `T.class_of(Foo)`' file='errors/errors.rb' line='5'>
321+
<failure type='7003'>
322+
<![CDATA[In file errors/errors.rb:
323+
5 | sig { params(bar: Bar).returns(C) }
324+
^^^
325+
Autocorrect: Use `-a` to autocorrect
326+
errors/errors.rb:5: Insert `extend T::Sig`
327+
5 | sig { params(bar: Bar).returns(C) }
328+
^]]>
329+
</failure>
330+
</testcase>
331+
<testcase name='Wrong number of arguments for constructor. Expected: `0`, got: `1`' file='errors/errors.rb' line='10'>
332+
<failure type='7004'>
333+
<![CDATA[In file errors/errors.rb:
334+
10 |b = Foo.new(42)
335+
^^
336+
https://github.com/sorbet/sorbet/tree/dad16b7fe5bf8ebdec30bac0d9c1cb0e66f582cf/rbi/core/basic_object.rbi#L228: `initialize` defined here
337+
228 | def initialize(); end
338+
^^^^^^^^^^^^^^^^
339+
Autocorrect: Use `-a` to autocorrect
340+
errors/errors.rb:10: Delete
341+
10 |b = Foo.new(42)
342+
^^]]>
343+
</failure>
344+
</testcase>
345+
<testcase name='Method `c` does not exist on `T.class_of(&lt;root&gt;)`' file='errors/errors.rb' line='11'>
346+
<failure type='7003'>
347+
<![CDATA[In file errors/errors.rb:
348+
11 |b.foo(b, c)
349+
^]]>
350+
</failure>
351+
</testcase>
352+
<testcase name='Too many arguments provided for method `Foo#foo`. Expected: `1`, got: `2`' file='errors/errors.rb' line='11'>
353+
<failure type='7004'>
354+
<![CDATA[In file errors/errors.rb:
355+
11 |b.foo(b, c)
356+
^
357+
errors/errors.rb:6: `foo` defined here
358+
6 | def foo(bar)
359+
^^^^^^^^^^^^
360+
Autocorrect: Use `-a` to autocorrect
361+
errors/errors.rb:11: Delete
362+
11 |b.foo(b, c)
363+
^^^]]>
364+
</failure>
365+
</testcase>
366+
</testsuite>
367+
XML
368+
assert_equal(expected_doc, @project.read("junit.xml"))
369+
refute(result.status)
378370
end
379371

380372
def test_pass_options_to_sorbet

0 commit comments

Comments
 (0)