Skip to content

Spec performance fixes - #1237

Open
apiology wants to merge 10 commits into
castwide:masterfrom
apiology:speed_up_specs_master
Open

Spec performance fixes#1237
apiology wants to merge 10 commits into
castwide:masterfrom
apiology:speed_up_specs_master

Conversation

@apiology

@apiology apiology commented Aug 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Spec performance fixes: replace full-project gem-pin caching with targeted single-gem caching in slow specs. That path is new - the old bulk-cache-everything approach never exercised it - and exposed two latent correctness bugs below, both required to make it work.

Spec performance

Replaced ApiMap.load_with_cache (builds pins for every gem) with ApiMap.load + targeted catalog(bench)/cache_gem for just the gems each test needs (api_map_method_spec.rb, pin/base_spec.rb, pin/method_spec.rb, rbs_map/conversions_spec.rb). Plus bundler-cache: true in CI and pinning bundler: 2.5.23 to match Gemfile.lock.

Correctness fixes (needed by the above)

Workspace::Gemspecs#gemspec_or_preference returned whatever spec type it was given instead of normalizing it:

# gemspec: Gem::Specification | Bundler::LazySpecification | Bundler::StubSpecification
def gemspec_or_preference(gemspec)
  return gemspec unless preference_map.key?(gemspec.name) # returned as-is, unnormalized
  ...
end

DocMap#cache gates on uncached_yard_gemspecs.include?(gemspec) - an equality check a mismatched type silently failed, no-op'ing cache_gem for specs from the new ApiMap#resolve_require. Fixed via to_gem_specification.

The pattern also requires catalog(bench) - which registers a gem as required - to run before cache_gem. Two specs had it backwards, silently caching nothing. Reordered both.

Minor

  • Disabled YARD/CollectionStyle (rubocop-yard autocorrect mangles nested-generic Hash tags).
  • Diagnostics#diagnose accepts an optional workspace: kwarg.
  • Removed a redundant mapper_spec.rb test.

@apiology apiology mentioned this pull request Aug 1, 2026
Closed
apiology
apiology commented Aug 2, 2026
View reviewed changes
Comment thread .rubocop.yml Outdated
@apiology
apiology marked this pull request as draft August 2, 2026 14:40
apiology added 6 commits August 2, 2026 12:56
@apiology
Rebase spec performance fixes onto master
82b2542 
This PR previously targeted v0.59; retargeting onto master. Squashed
the fork's net contribution (previously spread across the branch's
merge-heavy history against v0.59) into a single commit applied
cleanly on top of current castwide/master, resolving the one real
conflict in .github/workflows/plugins.yml (master's own
bundler-cache: false vs. this fork's bundler-cache: true perf fix for
run_solargraph_rails_specs).
@apiology
Fix ApiMap#resolve_require after rebase onto master
f6c351b 
Workspace#resolve_require no longer exists on master (that logic now
lives on Workspace::Gemspecs, per castwide's own refactor/revert
history). Route through Workspace::Gemspecs directly, matching the
pattern already used in spec/workspace/gemspecs_resolve_require_spec.rb.

Fixes the two api_map_method_spec.rb failures the "regression" CI job
caught (YAML/Psych and Thor.desc method-stack specs) - these were
masked as "pre-existing" in earlier local testing because that
comparison only checked before/after within the v0.59-based branch,
not across the base-branch move to master where this method moved.
@apiology
Suppress false-positive nilable-workspace typecheck nit in resolve_re…
dc17b43 
…quire
@apiology
Fix Thor.desc pin caching: gemspec type mismatch + test ordering
839ff36 
Root cause (two compounding bugs, both introduced by this branch's own
earlier perf work, not upstream castwide code):

1. Workspace::Gemspecs#gemspec_or_preference returned whatever spec type
   it was given (Gem::Specification, Bundler::LazySpecification, or
   Bundler::StubSpecification) without normalizing via
   to_gem_specification, despite its own @return [Gem::Specification]
   contract. ApiMap#resolve_require (which funnels through this method)
   was therefore returning Bundler::StubSpecification objects that don't
   == the Gem::Specification objects DocMap's own uncached_yard_gemspecs/
   uncached_rbs_collection_gemspecs tracking uses - so DocMap#cache's
   `uncached_yard_gemspecs.include?(gemspec)` check silently failed and
   cache_gem became a no-op.

2. spec/api_map_method_spec.rb's YAML and Thor tests called
   resolve_require + cache_gem *before* catalog(bench) - but catalog is
   what registers a gem as required in doc_map's internal tracking in
   the first place, so calling cache_gem first meant doc_map didn't yet
   know the gem needed caching. Reordered to catalog first.

The YAML test happened to keep passing throughout because it's stdlib,
cached via a separate always-on pathway (Ruby core RBS caching),
masking both bugs for that case.

Verified: reproduces and is fixed under both rbs 4.0.1 and 4.1.1;
full local suite (bundle exec rake spec) is 1616 examples, 0 failures.
@apiology
Fix same cache_gem-before-catalog ordering bug in pin/base_spec.rb
857d8f9 
Found while verifying this PR's original claimed accomplishments are
still intact. Same root cause as the Thor.desc fix: cache_gem was
called before catalog(bench) registered 'yard' as required, making the
cache a no-op. Confirmed via isolated fresh SOLARGRAPH_CACHE: failed
before this fix, passes after. It was masked in full-suite runs by
another spec warming yard's cache first in the same process - not
currently causing CI failures, but the same latent landmine.
@apiology
Note YARD version tested alongside rubocop-yard in CollectionStyle co…
ef278e4 
…mment
@apiology
apiology force-pushed the speed_up_specs_master branch from 4939fd0 to ef278e4 Compare August 2, 2026 16:57
apiology
apiology commented Aug 2, 2026
View reviewed changes
Comment thread lib/solargraph/api_map.rb
apiology
apiology commented Aug 2, 2026
View reviewed changes
Comment thread .github/workflows/rspec.yml Outdated
apiology added 4 commits August 2, 2026 14:49
@apiology
Address review comments: raise instead of sg-ignore, revert cosmetic …
070890e 
…reformat

- ApiMap#resolve_require: raise a clear error when called without a
  workspace instead of suppressing the nil-workspace typecheck warning
  with @sg-ignore.
- .github/workflows/rspec.yml: revert the undercover job's "Update
  types" step back to a single-line `run:` - the block-scalar form had
  identical content, a no-op reformat.
@apiology
Remove fork-only bundler version pin from rspec.yml
0cd6610 
Investigated why CI explicitly pinned `bundler: 2.5.23` (matching
Gemfile.lock's BUNDLED WITH) in both the main rspec matrix and
undercover jobs. This pin is fork-only - never merged to
castwide/master - introduced in 9d26863 "Fix new bundler issue" to
work around a failure specific to the `ruby-version: head` matrix
entry, which has since been removed entirely (unrelated 404 issue on
ubuntu-24.04, see the @todo above the matrix).

castwide/master's own rspec.yml has never had this pin and its CI
passes consistently (confirmed via recent successful runs). Also
reproduced locally: `bundle install` with the latest published bundler
(4.0.17, vs the pinned 2.5.23) against this project's Gemfile.lock
completes cleanly with no lockfile changes.

This brings rspec.yml back to an exact match with castwide/master,
removing it from this PR's diff. If CI still passes here, the pin was
dead weight from a since-resolved, no-longer-applicable issue.
@apiology
Revert "Remove fork-only bundler version pin from rspec.yml"
2ac9f9a 
This reverts commit 0cd6610.
@apiology
Document the confirmed reason for pinning bundler version in CI
492b000
@apiology
apiology marked this pull request as ready for review August 2, 2026 21:37
@apiology

apiology commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

@castwide - ready for review

apiology added a commit to apiology/solargraph that referenced this pull request Aug 3, 2026
@apiology
Disable YARD/CollectionStyle instead of reformatting
971dfd3 
Reverts the docstring reformatting from the previous commit: for
nested-generic/tuple Hash key types, rubocop-yard's long-style
autocorrect (Hash{Array(String, Array<String>) => ...} ->
Hash{Array, String, Array, String => ...}) produces syntax that
doesn't preserve the original tuple's meaning - it flattens the
tuple's nested types into an ambiguous flat list rather than valid
long-style syntax. Confirmed against rubocop-yard 1.3.0 / yard 0.9.45.

Disable the cop instead, matching the same finding and fix already
applied in castwide#1237.
@apiology apiology mentioned this pull request Aug 3, 2026
Draft
3 tasks
apiology added a commit to apiology/solargraph that referenced this pull request Aug 3, 2026
@apiology
Disable YARD/CollectionStyle; move all_params nil-safety fix out
d739226 
Two follow-ups to the earlier extraction:

- The per-file YARD/CollectionStyle todo exclusion was a stand-in for
  actually fixing the cop. Replace it with disabling the cop outright
  in .rubocop.yml, matching castwide#1237's fix: for
  nested-generic/tuple Hash key types, the cop's own long-style
  autocorrect produces syntax that doesn't preserve the original
  tuple's meaning, so there's no safe autocorrected form to converge
  on for those cases.
- UniqueType#to_rbs's all_params.empty? nil-safety fix is an
  unrelated, pre-existing bug (from the original branch's own
  history, unrelated to parallel specs) - move it to
  #43.
This was referenced Aug 3, 2026
Draft
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@apiology