Skip to content

Add Rspec included modules support #13

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.

Sign up for GitHub

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

Closed
ShadiestGoat wants to merge 6 commits into from
Closed

Add Rspec included modules support #13

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9ffb93e
Support Rspec.configure ... config.include
ShadiestGoat Jul 9, 2025
f4faace
Make tests pass
ShadiestGoat Jul 10, 2025
66d55fd
Require solargraph 56 (remote ci test)
ShadiestGoat Jul 10, 2025
7c817e7
Fix testing
ShadiestGoat Jul 13, 2025
49f22c1
Remove solargraph debug patch
ShadiestGoat Jul 16, 2025
1f8ac4b
Cleanup 1
ShadiestGoat Aug 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions .github/workflows/ruby.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ on:
permissions:
contents: read

env:
SOLARGRAPH_CACHE: "${{ github.workspace }}/solargraph-rspec/gem-cache"
BUNDLE_GEMFILE: "${{ github.workspace }}/solargraph-rspec/gemfiles/default.gemfile"

jobs:
test:
name: Tests (ruby v${{ matrix.ruby-version }})
Expand All @@ -25,7 +29,6 @@ jobs:
# FIXME: Why '3.0' is not working with Appraisal?
# FIXME: Add 'head' https://github.com/lekemula/solargraph-rspec/pull/8/commits/3b52752b96e7f2ec01831406f8e5a51c91523187
ruby-version: ['3.1', '3.2', '3.3']

steps:
- name: Checkout solargraph-rspec
uses: actions/checkout@v4
Expand All @@ -42,32 +45,33 @@ jobs:
- name: Cache Ruby gems
uses: actions/cache@v3
with:
path: solargraph-rspec/vendor/bundle
key: bundle-use-ruby-${{ matrix.os }}-${{ matrix.ruby-version }}-${{ hashFiles('solargraph-rspec/solargraph-rspec.gemspec') }}
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') }}
Copy link
Copy Markdown
Owner

@lekemula lekemula Jul 29, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@ShadiestGoat ShadiestGoat Aug 3, 2025

Choose a reason for hiding this comment

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

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


- name: Install dependencies
run: |
cd solargraph-rspec
bundle config path vendor/bundle
bundle install --jobs 4 --retry 3
bundle exec appraisal install

- name: Run Rubocop
run: cd solargraph-rspec && bundle exec rubocop
# - name: Run Rubocop
Copy link
Copy Markdown
Owner

@lekemula lekemula Jul 29, 2025

Choose a reason for hiding this comment

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

Please don't forget to bring this back again.

# run: cd solargraph-rspec && bundle exec rubocop

- 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: 'Apply solargraph debug patch'
Comment thread
lekemula marked this conversation as resolved.
Outdated
run: |-
echo 'exclude: ["vendor/**/*", "gemfiles/**/*"]' > solargraph-rspec/.solargraph.yml
cd solargraph-rspec/gemfiles/vendor/bundle/ruby/${{ matrix.ruby-version }}.0/gems/solargraph-0.56.0

- name: List all Yardoc constants and methods
run: |
cd solargraph-rspec
bundle exec yard list
awk '/Caching yardoc for/ { print "return nil unless Dir.exist?(gemspec.gem_dir)"; print; next }1' lib/solargraph/yardoc.rb > tmp.rb
mv tmp.rb lib/solargraph/yardoc.rb

- name: Cache Docs
run: |-
cd solargraph-rspec
bundle exec solargraph gems --rebuild

- name: Run tests
run: cd solargraph-rspec && bundle exec appraisal rspec --format progress
run: cd solargraph-rspec && bundle exec rspec --format progress
Copy link
Copy Markdown
Owner

@lekemula lekemula Jul 29, 2025

Choose a reason for hiding this comment

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

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.


- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v4
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@

# rspec failure tracking
.rspec_status

.solargraph.yml
vendor
gemfiles/vendor
1 change: 1 addition & 0 deletions gemfiles/.bundle/config
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
---
BUNDLE_RETRY: "1"
BUNDLE_PATH: "vendor/bundle"
54 changes: 30 additions & 24 deletions gemfiles/default.gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
PATH
remote: ..
specs:
solargraph-rspec (0.4.1)
solargraph (~> 0.52, >= 0.52.0)
solargraph-rspec (0.5.2)
solargraph (~> 0.56, >= 0.56.0)

GEM
remote: https://rubygems.org/
Expand Down Expand Up @@ -79,7 +79,7 @@ GEM
debug (1.10.0)
irb (~> 1.10)
reline (>= 0.3.8)
diff-lcs (1.6.0)
diff-lcs (1.6.2)
docile (1.4.1)
domain_name (0.6.20240107)
drb (2.2.1)
Expand All @@ -97,13 +97,13 @@ GEM
pp (>= 0.6.0)
rdoc (>= 4.0.0)
reline (>= 0.4.2)
jaro_winkler (1.6.0)
json (2.10.2)
jaro_winkler (1.6.1)
json (2.12.2)
kramdown (2.5.1)
rexml (>= 3.3.9)
kramdown-parser-gfm (1.1.0)
kramdown (~> 2.0)
language_server-protocol (3.17.0.4)
language_server-protocol (3.17.0.5)
lint_roller (1.1.0)
logger (1.6.6)
loofah (2.24.0)
Expand Down Expand Up @@ -134,16 +134,19 @@ GEM
netrc (0.11.0)
nokogiri (1.17.2-arm64-darwin)
racc (~> 1.4)
nokogiri (1.17.2-x86_64-linux)
racc (~> 1.4)
observer (0.1.2)
optparse (0.6.0)
ostruct (0.6.1)
parallel (1.26.3)
parser (3.3.7.2)
ostruct (0.6.2)
parallel (1.27.0)
parser (3.3.8.0)
ast (~> 2.4.1)
racc
pp (0.6.2)
prettyprint
prettyprint (0.2.0)
prism (1.4.0)
profile-viewer (0.0.4)
optparse
webrick
Expand Down Expand Up @@ -183,7 +186,7 @@ GEM
zeitwerk (~> 2.6)
rainbow (3.1.1)
rake (13.2.1)
rbs (3.6.1)
rbs (3.9.4)
logger
rdoc (6.12.0)
psych (>= 4.0.0)
Expand All @@ -200,16 +203,16 @@ GEM
reverse_markdown (3.0.0)
nokogiri
rexml (3.4.1)
rspec (3.13.0)
rspec (3.13.1)
rspec-core (~> 3.13.0)
rspec-expectations (~> 3.13.0)
rspec-mocks (~> 3.13.0)
rspec-core (3.13.3)
rspec-core (3.13.5)
rspec-support (~> 3.13.0)
rspec-expectations (3.13.3)
rspec-expectations (3.13.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-mocks (3.13.2)
rspec-mocks (3.13.5)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-rails (7.1.1)
Expand All @@ -225,20 +228,21 @@ GEM
rspec-expectations (~> 3.0)
rspec-mocks (~> 3.0)
sidekiq (>= 5, < 9)
rspec-support (3.13.2)
rubocop (1.74.0)
rspec-support (3.13.4)
rubocop (1.78.0)
json (~> 2.3)
language_server-protocol (~> 3.17.0.2)
lint_roller (~> 1.1.0)
parallel (~> 1.10)
parser (>= 3.3.0.2)
rainbow (>= 2.2.2, < 4.0)
regexp_parser (>= 2.9.3, < 3.0)
rubocop-ast (>= 1.38.0, < 2.0)
rubocop-ast (>= 1.45.1, < 2.0)
ruby-progressbar (~> 1.7)
unicode-display_width (>= 2.4.0, < 4.0)
rubocop-ast (1.41.0)
rubocop-ast (1.45.1)
parser (>= 3.3.7.2)
prism (~> 1.4)
ruby-progressbar (1.13.0)
securerandom (0.3.2)
shoulda-matchers (6.4.0)
Expand All @@ -258,28 +262,29 @@ GEM
simplecov (~> 0.19)
simplecov-html (0.13.1)
simplecov_json_formatter (0.1.4)
solargraph (0.52.0)
solargraph (0.56.0)
backport (~> 1.2)
benchmark
benchmark (~> 0.4)
bundler (~> 2.0)
diff-lcs (~> 1.4)
jaro_winkler (~> 1.6)
jaro_winkler (~> 1.6, >= 1.6.1)
kramdown (~> 2.3)
kramdown-parser-gfm (~> 1.1)
logger (~> 1.6)
observer (~> 0.1)
ostruct (~> 0.6)
parser (~> 3.0)
rbs (~> 3.0)
reverse_markdown (>= 2.0, < 4)
prism (~> 1.4)
rbs (~> 3.3)
reverse_markdown (~> 3.0)
rubocop (~> 1.38)
thor (~> 1.0)
tilt (~> 2.0)
yard (~> 0.9, >= 0.9.24)
yard-solargraph (~> 0.1)
stringio (3.1.5)
thor (1.3.2)
tilt (2.6.0)
tilt (2.6.1)
timeout (0.4.3)
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
Expand All @@ -298,6 +303,7 @@ GEM

PLATFORMS
arm64-darwin-24
x86_64-linux

DEPENDENCIES
actionmailer
Expand Down
17 changes: 16 additions & 1 deletion lib/solargraph/rspec/convention.rb
View file
Open in desktop
Copy link
Copy Markdown
Owner

@lekemula lekemula Jul 29, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

@ShadiestGoat ShadiestGoat Aug 2, 2025

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Contributor Author

@ShadiestGoat ShadiestGoat Aug 2, 2025

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

@lekemula lekemula Aug 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
require_relative 'correctors/subject_method_corrector'
require_relative 'correctors/context_block_methods_corrector'
require_relative 'correctors/dsl_methods_corrector'
require_relative 'spec_helper_include'
require_relative 'test_helpers'
require_relative 'pin_factory'

Expand Down Expand Up @@ -120,6 +121,8 @@ def local(source_map)
pins = []
# @type [Array<Pin::Namespace>]
namespace_pins = []
# @type [Array<String>]
extra_requires = ['rspec']

rspec_walker = SpecWalker.new(source_map: source_map, config: config)

Expand All @@ -133,14 +136,26 @@ def local(source_map)

rspec_walker.walk!
pins += namespace_pins
begin
pins += SpecHelperInclude.instance.pins
extra_requires += SpecHelperInclude.instance.extra_requires
rescue StandardError => e
Solargraph.logger.error("[solargraph-rspec] [spec helper] Can't add pins: #{e}")
Comment thread
ShadiestGoat marked this conversation as resolved.
Outdated
[]
end

if pins.any?
Solargraph.logger.debug(
"[RSpec] added #{pins.map(&:inspect)} to #{source_map.filename}"
)
end
if extra_requires.any?
Solargraph.logger.debug(
"[RSpec] added requires #{extra_requires} to #{source_map.filename}"
)
end

Environ.new(requires: [], pins: pins)
Environ.new(requires: extra_requires, pins: pins)
rescue StandardError, SyntaxError => e
raise e if ENV['SOLARGRAPH_DEBUG']

Expand Down
Loading