Filter Sorbet in backtrace #959

andyw8 · 2023-08-31T16:40:43Z

Paired on with @st0012

Motivation

When working on Ruby LSP, test failures contain Sorbet frames in the backtrace. This adds considerable noise and is generally not helpful for understand a failure.

Implementation

There is not much guidance in the Minitest documentation for this, so this solution is based on Minitest's default filter.

Automated Tests

n/a

Manual Tests

Add something to a test which triggers a error, and run the test. Ensure the backtrace filter does not include any Sorbet frames.

andyw8 · 2023-08-31T16:41:38Z

test/test_helper.rb

@@ -13,8 +13,8 @@
 require "debug"
 require "mocha/minitest"

-sorbet_paths = Gem.loaded_specs["sorbet-runtime"].full_require_paths.freeze
-DEBUGGER__::CONFIG[:skip_path] = Array(DEBUGGER__::CONFIG[:skip_path]) + sorbet_paths
+SORBET_PATHS = T.let(Gem.loaded_specs["sorbet-runtime"].full_require_paths.freeze, T::Array[String])


Changed to a constant that we can reference it from BacktraceWithoutSorbetFilter.

github-actions · 2023-08-31T16:53:19Z

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.210125 std_dev: 0.010702
          textDocument/diagnostic average: 0.048657 std_dev: 0.011258
          textDocument/definition average: 0.005104 std_dev: 0.002936
      textDocument/selectionRange average: 0.004462 std_dev: 0.000561
            textDocument/codeLens average: 0.002969 std_dev: 0.000496
      textDocument/documentSymbol average: 0.002862 std_dev: 0.000298
        textDocument/documentLink average: 0.002854 std_dev: 0.000462
 textDocument/semanticTokens/full average: 0.00285 std_dev: 0.000516
   textDocument/documentHighlight average: 0.002834 std_dev: 0.000404
        textDocument/foldingRange average: 0.002564 std_dev: 0.000344
textDocument/semanticTokens/range average: 0.001783 std_dev: 0.000211
               codeAction/resolve average: 0.001501 std_dev: 0.000303
           textDocument/inlayHint average: 0.001476 std_dev: 0.000202
               textDocument/hover average: 0.001461 std_dev: 0.000276
          textDocument/formatting average: 0.000934 std_dev: 0.000359
    textDocument/onTypeFormatting average: 0.000932 std_dev: 0.000142
          textDocument/codeAction average: 0.000899 std_dev: 0.000157


================================================================================
Comparison with main branch:

 textDocument/semanticTokens/full unchanged
textDocument/semanticTokens/range unchanged
      textDocument/documentSymbol unchanged
        textDocument/foldingRange unchanged
          textDocument/formatting unchanged
          textDocument/diagnostic unchanged
        textDocument/documentLink unchanged
           textDocument/inlayHint unchanged
      textDocument/selectionRange unchanged
   textDocument/documentHighlight unchanged
               textDocument/hover unchanged
          textDocument/codeAction unchanged
    textDocument/onTypeFormatting unchanged
               codeAction/resolve unchanged
          textDocument/completion unchanged
            textDocument/codeLens unchanged
          textDocument/definition unchanged


================================================================================
Missing benchmarks:

RubyLsp::Requests::ShowSyntaxTree

andyw8 · 2023-08-31T17:01:15Z

test/test_helper.rb

@@ -28,3 +28,20 @@ class Test
    Minitest::Test.make_my_diffs_pretty!
  end
 end
+
+# based on https://github.com/minitest/minitest/blob/master/lib/minitest.rb
+class BacktraceWithoutSorbetFilter < Minitest::BacktraceFilter


fyi @st0012 - I changed this so it inherits

vinistock · 2023-08-31T17:28:27Z

What happens if we have a runtime typechecking error? How does it show up?

andyw8 · 2023-08-31T17:40:30Z

It'll show like this:

DocumentSymbolExpectationsTest#test_document_symbol__keyword_splat_nil__does_not_raise:
TypeError: Parameter 'node': Expected type YARP::ModuleNode, got type YARP::DefNode with hash 2984306925210129830
Caller: /Users/andyw8/src/github.com/Shopify/ruby-lsp/lib/ruby_lsp/event_emitter.rb:120
Definition: /Users/andyw8/src/github.com/Shopify/ruby-lsp/lib/ruby_lsp/requests/document_symbol.rb:167
    /Users/andyw8/src/github.com/Shopify/ruby-lsp/lib/ruby_lsp/event_emitter.rb:120:in `block in visit_def_node

vinistock · 2023-08-31T17:42:42Z

test/test_helper.rb

+  def filter(bt)
+    return ["No backtrace"] unless bt
+
+    return bt.dup if $DEBUG


Why do we need this? When is $DEBUG true?

vinistock · 2023-08-31T17:44:48Z

test/test_helper.rb

+class BacktraceWithoutSorbetFilter < Minitest::BacktraceFilter
+  extend T::Sig
+  sig { override.params(bt: T.nilable(T::Array[String])).returns(T::Array[String]) }
+  def filter(bt)


Instead of replicating what Minitest is doing, can we not use super here?

def filter(bt) super.select { |line| SORBET_PATHS.none? { |path| line.include?(path) } } end

Good idea, updated.

vinistock · 2023-09-01T15:47:18Z

test/test_helper.rb

@@ -28,3 +28,15 @@ class Test
    Minitest::Test.make_my_diffs_pretty!
  end
 end
+
+class BacktraceWithoutSorbetFilter < Minitest::BacktraceFilter
+  extend T::Sig


Suggested change

extend T::Sig

extend T::Sig

andyw8 · 2023-09-01T15:57:16Z

We could potentially ship this with Spoom: Shopify/spoom#456

Co-authored-by: Andy Waite <[email protected]>

andyw8 commented Aug 31, 2023

View reviewed changes

andyw8 added the chore Chore task label Aug 31, 2023

andyw8 force-pushed the andyw8/filter-sorbet-in-backtrace branch 2 times, most recently from 7a933fa to acfe51b Compare August 31, 2023 16:59

andyw8 commented Aug 31, 2023

View reviewed changes

andyw8 marked this pull request as ready for review August 31, 2023 17:01

andyw8 requested a review from a team as a code owner August 31, 2023 17:01

andyw8 requested review from KaanOzkan and vinistock August 31, 2023 17:01

vinistock reviewed Aug 31, 2023

View reviewed changes

andyw8 force-pushed the andyw8/filter-sorbet-in-backtrace branch from acfe51b to 0e53c25 Compare September 1, 2023 15:45

vinistock approved these changes Sep 1, 2023

View reviewed changes

Filter Sorbet in backtrace

c6cdfee

andyw8 force-pushed the andyw8/filter-sorbet-in-backtrace branch from 0e53c25 to c6cdfee Compare September 1, 2023 15:52

andyw8 merged commit 1e2fdcc into main Sep 1, 2023
29 checks passed

andyw8 deleted the andyw8/filter-sorbet-in-backtrace branch September 1, 2023 15:52

andyw8 mentioned this pull request Sep 1, 2023

Minitest backtrace filter Shopify/spoom#456

Closed

andyw8 added a commit that referenced this pull request Sep 5, 2023

Filter Sorbet in backtrace (#959)

4812fc0

Co-authored-by: Andy Waite <[email protected]>

vinistock added a commit that referenced this pull request Feb 28, 2024

Always activate workspace if a custom Gemfile is configured (#959)

38b70ff

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Filter Sorbet in backtrace #959

Filter Sorbet in backtrace #959

andyw8 commented Aug 31, 2023 •

edited

Loading

andyw8 Aug 31, 2023

github-actions bot commented Aug 31, 2023 •

edited

Loading

andyw8 Aug 31, 2023

vinistock commented Aug 31, 2023

andyw8 commented Aug 31, 2023

vinistock Aug 31, 2023

vinistock Aug 31, 2023

andyw8 Sep 1, 2023

vinistock Sep 1, 2023

andyw8 commented Sep 1, 2023

Filter Sorbet in backtrace #959

Filter Sorbet in backtrace #959

Conversation

andyw8 commented Aug 31, 2023 • edited Loading

Motivation

Implementation

Automated Tests

Manual Tests

andyw8 Aug 31, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 31, 2023 • edited Loading

andyw8 Aug 31, 2023

Choose a reason for hiding this comment

vinistock commented Aug 31, 2023

andyw8 commented Aug 31, 2023

vinistock Aug 31, 2023

Choose a reason for hiding this comment

vinistock Aug 31, 2023

Choose a reason for hiding this comment

andyw8 Sep 1, 2023

Choose a reason for hiding this comment

vinistock Sep 1, 2023

Choose a reason for hiding this comment

andyw8 commented Sep 1, 2023

andyw8 commented Aug 31, 2023 •

edited

Loading

github-actions bot commented Aug 31, 2023 •

edited

Loading