Add Rspec included modules support by ShadiestGoat · Pull Request #13 · lekemula/solargraph-rspec

ShadiestGoat · 2025-07-08T22:39:03Z

This doesn't work atm. @lekemula could you take a look?

The goal of this is to grab everything that is included in Rspec.configure, and then include them under examples, example groups & hooks. Afaik, through the power of ❇️ inheritence ❇️, including them in the example group classes (& requiring the needed files) should "just work". I've tried to use the 4 namespaces I know of, but please let me know if I am way off base?

As a side note - I took the design of these processors directly from solargraph-rails so thanks to the maintainer <3

ShadiestGoat · 2025-07-08T22:44:35Z

As another side note - this branch is full of test/debug code, which I'm using for testing this on the spec of this project (not working lmao)

Also, to get ANYTHING working for this, I had to set my solargraph's exclude to be spec/solargraph/**/* rather than spec/**/*.rb

ShadiestGoat · 2025-07-09T11:07:30Z

Looks like I got it working, except for spec

ShadiestGoat · 2025-07-10T01:21:54Z

After hours of deep solargraph diving, I realized I had a bad rspec stub, specs are now happy & this PR is ready for review!

ShadiestGoat · 2025-07-11T21:49:38Z

Ok, I'm finally able to replicate these CI failures via docker file, looking into them

codecov · 2025-07-13T11:31:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.84%. Comparing base (1cf43a3) to head (49f22c1).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #13      +/-   ##
==========================================
+ Coverage   86.54%   88.84%   +2.29%     
==========================================
  Files          23       25       +2     
  Lines        1078     1156      +78     
  Branches       77       85       +8     
==========================================
+ Hits          933     1027      +94     
+ Misses        145      129      -16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ShadiestGoat · 2025-07-13T12:46:35Z

Hey @lekemula, looks like I got it working, though its more changes that I would've liked.

Library Changes

Breaking: requires solargraph 0.56. This was needed to do get some tests to pass, but idk if its actually required. Lmk if this is a problem & you want me to investigate getting it to work w/ 0.52

Otherwise, the following is added:

rspec is required in every spec file
All (present) helper files are required spec/spec_helper.rb, spec/rails_helper.rb & spec/support/**/*.rb.
Also added support for include in those helper files

Eg.

module ModuleName
    def example_method
    end
end

Rspec.configure do |any_var|
    any_var.include ModuleName
end

Will include ModuleName in every example context & example. This means that example_method definition will be present in them.

CI Changes

I upgraded some gems in appraisal gemlock.
Set the bundle path to be a local one in the appraisal bundle config.
Caching hash change to include only the appraisal gemfile & gem lock
Changed gem doc parsing from yard to solargraph
- Theres a patch I included that may no longer be needed, bundler was kinda breaking this command but with the other changes, idk if this is needed. Lmk if you want me to experiment removing this
- I called --rebuild here & I'm not caching the results, since otherwise, for some reason, rspec docs are not taken into account. As you can see from the PR history, I really tried to get it to work, but it just really doesn't like me ig
I removed the whole appraisal part - I'm basically simulating it via the BUNDLE_GEMFILE env var the top, which is what appraisal does anyways.
- This also includes only installing the appraisal gems

lekemula · 2025-07-29T20:33:30Z

-
      - name: Cache Docs
        run: |-
+          # Vendor files: don't do them


Can we add more context on why we don't need vendor files? My understanding is that it is because we run specs via appraisal gem, is that correct?

Bad comment on my part - we still have vendor files, just they're excluded in a different way in the sg workspace. By default, only vendor/** get excluded. The only change is **/vendor/**/*, ie. "Exclude vendor files everywhere, not just in the top dir"

lekemula · 2025-07-29T20:37:43Z

-          restore-keys: |
-            bundle-use-ruby-${{ matrix.os }}-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/solargraph-rspec.gemspec') }}
+          path: solargraph-rspec/gemfiles/vendor/bundle
+          key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'solargraph-rspec/gemfiles/default.gemfile.lock') }}


Good catch, but I think we need to check solargraph-rspec.gemspec, and solargraph-rspec/Gemfile, and Appraisals file instead.

Suggested change

key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'solargraph-rspec/gemfiles/default.gemfile.lock') }}

key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'Appraisals') }}

The solargraph-rspec/gemfiles/default.gemfile.lock is just a product of the Appraisals file, where in the future there could potentially be more dependency versions.

I'm actually not so sure - the gemlock file is the source of truth on pinned gems for tests, which could be updated without either of those 2 changing

lekemula · 2025-07-29T20:38:16Z

-      - name: Set up yardocs
-        # yard gems caches the yardocs into <gem_path>/doc/.yardoc path, hence they should be cached by ruby gems cache 
-        run: cd solargraph-rspec && bundle exec appraisal yard gems --verbose
+      # - name: Run Rubocop


Please don't forget to bring this back again.

lekemula · 2025-07-29T20:39:47Z


      - name: Run tests
-        run: cd solargraph-rspec && bundle exec appraisal rspec --format progress
+        run: cd solargraph-rspec && bundle exec rspec --format progress


I removed the whole appraisal part - I'm basically simulating it via the BUNDLE_GEMFILE env var the top, which is what appraisal does anyways.

I'd prefer to keep the Appraisal gem in place. While I understand that you're replicating its behavior under the hood, relying on the gem provides a clear standard and well-maintained documentation. This makes it easier for other contributors to understand how things work without needing to dig into custom logic.

lekemula · 2025-07-29T20:43:43Z

important: These changes are more appropriate for the Convention#global method rather than Convention#local, since RSpec.configure applies global configuration.

Additionally, this approach helps avoid redundant requires and pins in individual spec files, which could unnecessarily increase Solargraph's memory usage.

True - the one concern I have (which btw, not done right now) is "how do we handle updates to this file"? Imo the simple and easy approach is to call reset if the file name matches one of the configured paths but idk how I'd propagate that to the rest of the files (even via global)

Imo its not a HUGE issue since those files are edited rarely (let alone to include a module) but still, something to think about

True - the one concern I have (which btw, not done right now) is "how do we handle updates to this file"?

That's a very good point that I overlooked. As you said, these files tend not to change often, and I also don't find that to be the end of the world. One in theory could fix that by restarting the LSP, albeit not an ideal solution.

What we could do, though, is to include these pins in #local when the current file paths (ie. source_map.filename) match:

COMMON_HELPER_FILES = [ 'spec/spec_helper.rb', 'spec/rails_helper.rb' ].freeze

I think/hope this would ensure that whenever these files are changed, we would get the latest includes on the spec files. Can we try this?

If it does not work, I'd still go with #global first and seek a suggestion on the solargraph side. Maybe @castwide could have other ideas.

lekemula · 2025-07-29T20:52:59Z

+      COMMON_HELPER_FILES = [
+        'spec/spec_helper.rb',
+        'spec/rails_helper.rb'
+      ].freeze


Suggestion: Make these configurable and move to Solargraph::Rspec::Config#config_helper_files. This would allow users to add more files as needed.

lekemula · 2025-07-29T20:56:29Z

+      end
+
+      def extra_requires
+        included_modules.map(&:file).uniq + Dir['spec/support/**/*.rb']


All (present) helper files are required spec/spec_helper.rb, spec/rails_helper.rb & spec/support/**/*.rb.

If we do not exclude the spec/support/**/*.rb, is the additional require needed?

lekemula · 2025-07-29T21:10:53Z

+      # @param file [String]
+      #
+      # @return [Array<INCLUDED_MODULE_DATA>]
+      def extract_included_modules(ast, file)


Can we please extract this into a: Solargraph::Rspec::SpecConfigurationWalker, similar to lib/solargraph/rspec/spec_walker.rb?

I would prefer to keep everything isolated within that module, while keeping the plugin/pin logic in this class. That has proven helpful when switching between different parsers last time.

What do you think about having a spec ast module? It'll include the normal walker, the spec walker & the new config walker?

I'd prefer keeping unit tests matching the file names and separated. It makes it easier to navigate between source/test files (I use https://github.com/tpope/vim-projectionist) or run them via guard (which I plan to add in the future).

For integration-like specs (ie. test cases in convention_spec.rb), I'm more relaxed having specs which don't match the file name and rather be named by the functionality under test or a category of tests. convention_spec.rb ought to be split rather soon, since it has grown quite a bit already, and 3rd party plugins tests are a good candidate for extraction for example.

Hey sorry - I think I worded another thing badly. I meant a module for walkers in general, which would include the spec walker that already exists & a new class for the rspec config walker (and, in the future, a factory bot walker)

Oh yeah, that would make totally sense! 💯

lekemula · 2025-07-29T21:15:30Z

As another side note - this branch is full of test/debug code, which I'm using for testing this on the spec of this project (not working lmao)

Also, to get ANYTHING working for this, I had to set my solargraph's exclude to be spec/solargraph/**/* rather than spec/**/*.rb

Thanks for the notes, @ShadiestGoat — that approach seemed to work while testing in this repo. 💪

That said, I think we might have a potential issue. It’s not something we necessarily need to solve right now, but it’s worth noting:

Ideally, we want specs to be parsed on-demand to avoid Solargraph indexing delays. That’s why excluding spec/**/*.rb is preferred.
However, many Rails apps do not namespace their specs under spec/<app_name>, so this exclusion pattern could become a limitation and lead to initial solargraph indexing delays (especially on the large apps).
What we likely need is a smarter way on the Solargraph side to include a subdirectory (e.g., spec/support/**/*.rb) while still excluding spec/**/*.rb broadly.

Might be worth exploring options or workarounds in Solargraph’s configuration down the line.

ShadiestGoat · 2025-08-02T19:36:55Z

As another side note - this branch is full of test/debug code, which I'm using for testing this on the spec of this project (not working lmao)
Also, to get ANYTHING working for this, I had to set my solargraph's exclude to be spec/solargraph/**/* rather than spec/**/*.rb

Thanks for the notes, @ShadiestGoat — that approach seemed to work while testing in this repo. 💪

That said, I think we might have a potential issue. It’s not something we necessarily need to solve right now, but it’s worth noting:
* Ideally, we want specs to be parsed on-demand to avoid Solargraph indexing delays. That’s why excluding `spec/**/*.rb` is preferred.

* However, many Rails apps do not namespace their specs under `spec/<app_name>`, so this exclusion pattern could become a limitation and lead to initial solargraph indexing delays (especially on the large apps).

* What we likely need is a smarter way on the Solargraph side to include a subdirectory (e.g., `spec/support/**/*.rb`) while still excluding `spec/**/*.rb` broadly.
Might be worth exploring options or workarounds in Solargraph’s configuration down the line.

I 100% agree on your rails point. In the app I test this stuff on (~250 app files, ~600 files including spec) I had to change the inclusion patter to look something like:

include:
- app/**/*.rb
- lib/**/*.rb
- db/schema.rb
- spec/*.rb
- spec/support/**/*.rb

But thats not really ideal to ask a user to do, but idk this seems like more of a solargraph problem

ShadiestGoat · 2025-08-02T19:38:15Z

(I've switched to draft mode till I get this back into a reviewable state after the requested changes)

Credits to ShadiestGoat for #13 I mostly cherry-picked the changes from there and addressed my own feedback with the help of Claude Code. Co-authored-by: Lucy Dryaeva <48590492+ShadiestGoat@users.noreply.github.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Credits to ShadiestGoat for #13 I mostly cherry-picked the changes from there and addressed my own feedback with the help of Claude Code. Co-authored-by: Lucy Dryaeva <48590492+ShadiestGoat@users.noreply.github.com>

Credits to ShadiestGoat for #13 I mostly cherry-picked the changes from there and addressed my own feedback with the help of Claude Code. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Lucy Dryaeva <48590492+ShadiestGoat@users.noreply.github.com>

* Add Rspec included modules support Credits to ShadiestGoat for #13 I mostly cherry-picked the changes from there and addressed my own feedback with the help of Claude Code. * Update Changelog * Update README Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Lucy Dryaeva <48590492+ShadiestGoat@users.noreply.github.com>

lekemula · 2026-02-27T22:04:59Z

Hi @ShadiestGoat, I was keen to move this forward quickly, so I hope you don't mind me stepping in to address the remaining feedback. Really appreciate your contributions here 🙏

#32

I might do the same with #14 unless you plan to work on it soon, because that would be a HUGE feature! 🚀

ShadiestGoat · 2026-02-28T00:27:57Z

Thank you, I totally forgot about this.

I'd love to finish up #14, I'll be free on Sunday/sat evening to finish it up. It's actually a new years resolution of mine to do more oss stuff, so I'd be glad to finish!

lekemula · 2026-02-28T16:05:03Z

Thank you, I totally forgot about this.

I'd love to finish up #14, I'll be free on Sunday/sat evening to finish it up. It's actually a new years resolution of mine to do more oss stuff, so I'd be glad to finish!

@ShadiestGoat, I saw in your profile that Ruby was not one of your backend languages, so I thought maybe you moved to something else. But glad to hear that you plan to stick with us for a little longer ❤️

ShadiestGoat · 2026-02-28T16:41:21Z

Ha! It's not one of my preferred languages - I like my languages strict, but I do use ruby at work on a daily basis so what can you do. I have grown to like a lot of it's weirdness though haha

ShadiestGoat force-pushed the add-include-support branch 2 times, most recently from 62ef359 to b984303 Compare July 8, 2025 22:41

Support Rspec.configure ... config.include

9ffb93e

ShadiestGoat force-pushed the add-include-support branch from b984303 to 9ffb93e Compare July 9, 2025 11:06

ShadiestGoat force-pushed the add-include-support branch from bb61da0 to 55a2596 Compare July 10, 2025 01:20

ShadiestGoat changed the title ~~[WIP] Add include support~~ Add Rspec included modules support Jul 10, 2025

Make tests pass

f4faace

ShadiestGoat force-pushed the add-include-support branch from 55a2596 to f4faace Compare July 10, 2025 02:04

Require solargraph 56 (remote ci test)

66d55fd

ShadiestGoat force-pushed the add-include-support branch from 7f7e1e1 to 66d55fd Compare July 10, 2025 14:17

ShadiestGoat force-pushed the add-include-support branch 2 times, most recently from f47db3a to c68d2dc Compare July 12, 2025 15:27

Fix testing

7c817e7

ShadiestGoat force-pushed the add-include-support branch from 7073419 to 7c817e7 Compare July 13, 2025 12:10

lekemula reviewed Jul 15, 2025

View reviewed changes

Comment thread .github/workflows/ruby.yml Outdated

Remove solargraph debug patch

49f22c1

ShadiestGoat force-pushed the add-include-support branch 2 times, most recently from 9d4d3f2 to 49f22c1 Compare July 16, 2025 11:28

lekemula reviewed Jul 29, 2025

View reviewed changes

Cleanup 1

1f8ac4b

ShadiestGoat marked this pull request as draft August 2, 2025 19:37

lekemula mentioned this pull request Feb 27, 2026

Add Rspec included modules support #32

Merged

lekemula closed this Feb 27, 2026

	key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'solargraph-rspec/gemfiles/default.gemfile.lock') }}
	key: bundle-use-ruby-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/Gemfile', 'Appraisals') }}

Conversation

ShadiestGoat commented Jul 8, 2025

Uh oh!

ShadiestGoat commented Jul 8, 2025

Uh oh!

ShadiestGoat commented Jul 9, 2025

Uh oh!

ShadiestGoat commented Jul 10, 2025

Uh oh!

ShadiestGoat commented Jul 11, 2025

Uh oh!

codecov Bot commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ShadiestGoat commented Jul 13, 2025

Library Changes

CI Changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lekemula Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lekemula Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lekemula commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShadiestGoat commented Aug 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShadiestGoat commented Aug 2, 2025

Uh oh!

lekemula commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ShadiestGoat commented Feb 28, 2026

Uh oh!

lekemula commented Feb 28, 2026

Uh oh!

ShadiestGoat commented Feb 28, 2026

Uh oh!

Reviewers

Assignees

Labels

codecov Bot commented Jul 13, 2025 •

edited

Loading

lekemula Aug 3, 2025 •

edited

Loading

lekemula Aug 3, 2025 •

edited

Loading

lekemula commented Jul 29, 2025 •

edited

Loading

ShadiestGoat commented Aug 2, 2025 •

edited

Loading

lekemula commented Feb 27, 2026 •

edited

Loading