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 ruby-lsp-check executable #811

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Add ruby-lsp-check executable #811

merged 1 commit into from
Jul 12, 2023

Conversation

vinistock
Copy link
Member

Motivation

The easiest way for us to verify that a release didn't break anything critical is to run our requests against large codebases with numerous examples of syntax.

Having an executable that we can easily invoke to check if everything is fine before releasing should assist us in finding bugs ahead of time.

Implementation

The executable lists every file in the project and then executes one of our automatic requests. In the new listener pattern, it means all automatic requests are executed with it and so any issues with these critical requests will be surfaced.

Manual Tests

  1. Switch to this branch
  2. Go to any project you'd like to use the executable on
  3. Run /path/to/ruby-lsp-check in the project
  4. Verify that it runs fine

@vinistock vinistock added the enhancement New feature or request label Jul 12, 2023
@vinistock vinistock added this to the 2023-Q3 milestone Jul 12, 2023
@vinistock vinistock self-assigned this Jul 12, 2023
@vinistock vinistock requested a review from a team as a code owner July 12, 2023 15:36
@st0012
Copy link
Member

st0012 commented Jul 12, 2023

Do we want to run this against ruby-lsp on CI to make sure it doesn't break unnoticed?

@github-actions
Copy link
Contributor

github-actions bot commented Jul 12, 2023

Benchmark results in seconds (slowest at top)

          textDocument/completion average: 0.322845 std_dev: 0.012296
          textDocument/diagnostic average: 0.056412 std_dev: 0.012397
          textDocument/definition average: 0.005789 std_dev: 0.003421
      textDocument/selectionRange average: 0.004504 std_dev: 0.000706
        textDocument/documentLink average: 0.002393 std_dev: 0.000455
   textDocument/documentHighlight average: 0.002374 std_dev: 0.00032
            textDocument/codeLens average: 0.00231 std_dev: 0.000438
      textDocument/documentSymbol average: 0.002222 std_dev: 0.000516
 textDocument/semanticTokens/full average: 0.002192 std_dev: 0.000476
        textDocument/foldingRange average: 0.002014 std_dev: 0.000391
textDocument/semanticTokens/range average: 0.001084 std_dev: 0.00035
               codeAction/resolve average: 0.000787 std_dev: 0.000153
           textDocument/inlayHint average: 0.000757 std_dev: 0.000345
               textDocument/hover average: 0.000732 std_dev: 0.000143
    textDocument/onTypeFormatting average: 9.7e-05 std_dev: 4.2e-05
          textDocument/formatting average: 9.6e-05 std_dev: 0.000403
          textDocument/codeAction average: 5.5e-05 std_dev: 5.0e-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

@vinistock
Copy link
Member Author

Yes, we definitely should! I added a new step.

Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

I think this increases the average CI job time from 1.5~2 min to 5 min+

Do we want to move it to a different job so it runs parallel with tests?

main

Screenshot 2023-07-12 at 18 14 52

This branch

Screenshot 2023-07-12 at 18 15 21

exe/ruby-lsp-check Outdated Show resolved Hide resolved
@vinistock
Copy link
Member Author

Yeah, good point. There's no reason to run it on the entire matrix of Ruby versions either. I split it into a separate action.

@vinistock vinistock merged commit beb39e4 into main Jul 12, 2023
25 checks passed
@vinistock vinistock deleted the vs/add_lsp_check branch July 12, 2023 18:27
@shopify-shipit shopify-shipit bot temporarily deployed to production July 14, 2023 16:35 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants