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

Do not try to generate test command for temporary files #945

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

vinistock
Copy link
Member

Motivation

For unsaved files, the URI has no path and therefore there's no way to run tests defined in them. Currently, we're breaking when generating the test command.

Implementation

We need to check if there's a path before trying to generate test commands or code lens.

Automated Tests

Added an example that reproduces the problem.

@vinistock vinistock added the bugfix This PR will fix an existing bug label Aug 29, 2023
@vinistock vinistock added this to the 2023-Q3 milestone Aug 29, 2023
@vinistock vinistock self-assigned this Aug 29, 2023
@vinistock vinistock requested a review from a team as a code owner August 29, 2023 15:58
@github-actions
Copy link
Contributor

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.206018 std_dev: 0.003755
          textDocument/diagnostic average: 0.043393 std_dev: 0.009991
          textDocument/definition average: 0.004942 std_dev: 0.002871
      textDocument/selectionRange average: 0.004248 std_dev: 0.00062
   textDocument/documentHighlight average: 0.002652 std_dev: 0.000266
        textDocument/documentLink average: 0.002561 std_dev: 0.000297
 textDocument/semanticTokens/full average: 0.002528 std_dev: 0.000343
            textDocument/codeLens average: 0.002523 std_dev: 0.000251
      textDocument/documentSymbol average: 0.00252 std_dev: 0.00017
        textDocument/foldingRange average: 0.002476 std_dev: 0.000145
textDocument/semanticTokens/range average: 0.001645 std_dev: 9.9e-05
               codeAction/resolve average: 0.00148 std_dev: 0.000129
               textDocument/hover average: 0.001422 std_dev: 0.000135
           textDocument/inlayHint average: 0.001418 std_dev: 0.000152
    textDocument/onTypeFormatting average: 0.000854 std_dev: 0.000101
          textDocument/formatting average: 0.000837 std_dev: 0.000291
          textDocument/codeAction average: 0.000825 std_dev: 8.8e-05


================================================================================
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

@@ -89,7 +89,7 @@ def on_def(node)
visibility, _ = @visibility_stack.last
if visibility == "public"
method_name = node.name.value
if method_name.start_with?("test_")
if @path && method_name.start_with?("test_")
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) could this be be part of the return unless... guard above?

@vinistock vinistock merged commit 7f26495 into main Aug 29, 2023
30 of 31 checks passed
@vinistock vinistock deleted the vs/fix_code_lens_for_temporary_files branch August 29, 2023 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants