Allow ENV to override GlobalState#test_library

[petekinnecom]

Motivation

In our project we have a small amount of rspec tests and a large amount of minitest tests. The test library detection method prefers rspec over minitest and as a result, the testing code lenses don't work for the vast majority of our tests.

Having multiple test frameworks in-use in a single project is certainly not standard. Ours is a monorepo-ish application born years ago, being converted from engines to packwerk, so it's got a sprinkling of various things in it. We're looking for some way to be able to override the test library detection to be able to force it to "rails".

Implementation

Potential solutions I've dreamed up:

1. Create our own custom Addon that inherits from RubyLsp::Rails::Addon but removes this restriction: https://github.com/Shopify/ruby-lsp-rails/blob/8c0e73a2e938c5cda5e7ff20098d78d132f09853/lib/ruby_lsp/ruby_lsp_rails/addon.rb#L50

• Pros: No upstream changes
• Cons: Our little anemic Addon has to be committed and maintained over time

2. Same as (1) except we fork the RubyLsp::Rails repo and remove the line.

3. Allow an ENV var to specify the testing library

• Pros: Small code change
• Cons: one-off ENV var specification rather than being specified as part of a wider configuration system. (Not sure if this other open PR would be relevant here or if it's tackling a different sort of configuration? Introduce .ruby-lsp.yml config file #1434)

4. Add some way to hook into the loading of RubyLsp so that I can go wild with monkey-patching behavior modifications. Something like RubyLsp looks for ruby-lsp.rb or something at the root dir and just loads it as part of its booting process. In my case, I could use it to prepend my own module to the library, but I could also see this as a way to make small Addons where creating a full gem seems like a lot. Sort of a use-at-your-own-risk escape hatch.

• Pros: Small code change.
• Cons: Increased surface for issues to arise. Ie, how many new issues will be created in your repo because somebody checked in a broken boot file into their codebase. Those codebases descend into chaos and darkness (though perhaps a few reach enlightenment).

5. I create ruby-lsp.rb in my home directory that uses that uses the const_defined callback to modify RubyLsp::GlobalState as its loaded. Then I export RUBYOPT="-r ${HOME}/ruby-lsp" to get my fingers into every single ruby process and I convince my coworkers to do the same.

• Pros: I bask in the glory of my rube-goldberg creation
• Cons: too many to list

In this PR I've gone with option 3.

Though again, I can understand if this whole minitest/rspec living together is not a use-case you care to support. Which is one reason I actually kind of want option 4 (some way to hook into the the various ruby-lsp processes being run). It would be a simple outlet for us if you don't want to support things like this and it would enable us to deal with any other issues that arise immediately while we see about upstreaming solutions.

Automated Tests

I added the test. It's a bit annoying to mutate the ENV and put it back correctly. It's also not thread-safe. But it works.

Manual Tests

Make a rails project that contains both rspec and minitest gems and tests. The test runners will be present for the rspec tests and not for the minitest tests. Then, export RUBY_LSP_TEST_LIBRARY="minitest" and reload the window. The opposite will now be true.

[petekinnecom]

I have signed the CLA!

[vinistock]

Thank you for the contribution!

Unfortunately, using an environment variable doesn't work well for all editors. For example, it would not be easy to set that variable for VS Code, especially if you're switching projects without closing the editor. In general, configuration for language servers must be done through the initialization options received during the initialize request.

But I'd argue against adding a new setting even if this was implemented using initialization options. There are a few reasons for that:

1. We make an explicit effort to reduce the amount of configuration we add as much as possible. The more configuration that exist to make things work rather than for customizing behaviour, the more difficult it becomes to provide a nice experience out of the box
2. Every new setting we add, we must continue maintaining or cause breakages for users - since they will start relying on that new setting
3. As you pointed out, using two test frameworks on the same project is not very usual
4. The appropriate fix is to check for the ancestors of the test class to determine if it inherits from Minitest::Test, ActiveSupport::TestCase, Test::Unit::TestCase and otherwise fallback to RSpec. This is tracked in Use ancestors to determine test framework in code lens #1334. That way code lens will work out of the box without requiring any configuration, even in situations like these

Concerning the scenarios presented, we're working to make addons the recommended way of enhancing the Ruby LSP's base functionality, so we wouldn't be open to creating an alternative method of doing it.

[petekinnecom]

Yeah, no worries, that all makes sense. And I didn't see that PR which you linked which is clearly the better solution in the end! :)

One reason I was looking around for another way to hook in was to have a way to "beta" test my own addon across various repos. My understanding is that in order to install and use an addon, it either requires adding it to the app's Gemfile (which forces it on everyone) or having an external "custom" rubyLsp.bundleGemfile (in the vscode extension) which loses some of the goodness of the automated plugin (since it's no longer in the context of my application, if I'm understanding it all correctly). But I will play around with things and follow-up in the slack channel if needed. Thanks!

[vinistock]

Yeah, we're still thinking about how to deliver a smooth experience with addons. One option might be configuring the desired addons from the editor.