Skip to content

Reproduce solargraph-rspec issue#1107

Closed
apiology wants to merge 35 commits intocastwide:masterfrom
apiology:repro_solargraph_rspec_issue
Closed

Reproduce solargraph-rspec issue#1107
apiology wants to merge 35 commits intocastwide:masterfrom
apiology:repro_solargraph_rspec_issue

Conversation

@apiology
Copy link
Contributor

@apiology apiology commented Oct 2, 2025

I hit a failure in the solargraph-rspec specs while putting together a new merge branch. It looks like it can be reproduced with the combination of:

Any two together do not reproduce it.

apiology and others added 30 commits September 25, 2025 11:15
YARD-parsed namespaces weren't correctly setting their gates, leading
to unresolved types from methods.
Also replaces 'include' logic with call to ApiMap::Constants

Fixes castwide#1099
1) Prefer closures with more gates, to maximize compatibility
2) Look at basename of location, to make choice consistent
This takes out some lower value combinations - ideally we could keep
the number of jobs to <= 20, which is the max that GHA will run
simultaneously here.
Found in the Solargraph::Pin::DelegatedMethod class in strong
typechecking in #12
@apiology
Copy link
Contributor Author

apiology commented Oct 2, 2025

Heads up @lekemula - will see if I can figure out what's going on. Will be a good excuse to get set up with solargraph-rspec!

apiology added a commit to apiology/solargraph-rspec that referenced this pull request Oct 2, 2025
In an upcoming solargraph PR (see
castwide/solargraph#1107 as an example), we
won't be able to rely on resolving RSpec constants whose gem names we
don't provide using the 'require' convention.

That said, we also should not be caching as aggressively, either - so
I'd recommend dropping this spec workaround, which seems to resolve
the issue seen in the PR above.
@lekemula
Copy link
Contributor

lekemula commented Oct 2, 2025

Thank you so much @apiology for being cautious here - this would be way more difficult to resolve if we would have noticed it at some later point.

Will be a good excuse to get set up with solargraph-rspec!

Absolutely! @castwide, I would appreciate it if we could merge #1087 in, sooner rather than later, to prevent more cases like this. 🙏

@apiology
Copy link
Contributor Author

apiology commented Oct 2, 2025

OK, I believe that this is a false alarm - lekemula/solargraph-rspec#26 is a spec-only change on the solargraph-rails side that should resolve it. If @lekemula agrees, we can rerun the test workflow here once the PR is merged.

apiology added a commit to apiology/solargraph-rspec that referenced this pull request Oct 5, 2025
This matches a future Solargraph change which seems to be more
conservative including gems while resolving constants - see
castwide/solargraph#1107 for an example
failure.
@apiology
Copy link
Contributor Author

apiology commented Oct 5, 2025

@lekemula and I are closing in on a fix; I'll close this as it doesn't reflect an issue on the Solargraph side as far as I can see.

@apiology apiology closed this Oct 5, 2025
lekemula pushed a commit to lekemula/solargraph-rspec that referenced this pull request Oct 11, 2025
This matches a future Solargraph change which seems to be more
conservative including gems while resolving constants - see
castwide/solargraph#1107 for an example
failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants