Spec performance fixes - #1158
Closed
apiology wants to merge 22 commits into
Closed
Conversation
On my machine, I got a 33% speed-up with these tweaks: before: Finished in 1 minute 15.88 seconds after: Finished in 49 seconds
apiology
commented
Jan 15, 2026
| expect(pins.map(&:return_type).uniq.map(&:to_s)).to eq(['self']) | ||
| expect(pins.flat_map(&:signatures).map(&:return_type).uniq.map(&:to_s)).to eq(['self']) | ||
| end | ||
|
|
Contributor
Author
There was a problem hiding this comment.
This is slow and doesn't describe how it tests anything different than the spec above it
apiology
marked this pull request as draft
January 16, 2026 03:30
apiology
force-pushed
the
speed_up_specs
branch
from
January 16, 2026 12:02
8ba7f22 to
ffc25d6
Compare
* Fix merge * Adjust annotations * Fix typecheck errors * Fix merge * Fix merge * Fix some @sg-ignores * Fix merge * Merge branch 'intersection_types' into flow_sensitive_typing_2_0 * Fix RuboCop issue * Fix type issues * Fix type issues * Fix rspec.yml * Fix rspec.yml * Add @sg-ignores * Remove @sg-ignores * Merge branch 'or_support_in_flow_sensitive_typing' into union_type_enforcement * Fix spec * Fix annotations * Fix RuboCop issues * Bump RBS versions in rspec test * Fix version * Fix version matrix * Fix version matrix * Fix version matrix * Fix version matrix * Fix version matrix * Exclude another * Exclude another * Add version, fix doc * init -> config * Fix rbs-version for Ruby 4.0 in CI workflow * Clean up ruby-version entries in rspec.yml Removed deprecated ruby-version entries for RBS. * Fix RBS version for Ruby 4.0 in workflow * Fix merge * Improve signature combination Use our generated RBS signature from parameters as a key to combine method signatures from RBS/YARD pins. This is closer to what RBS does than the current technique of using the arity alone, and fixes a key degenerate case in Integer#+ revealed by updated definitions used by recently released RBS gems * Update annotations * Drop annotation * Fix RuboCop issue * Fix merge * Don't use solargraph-rspec branch * Fix merge * Debug * Add another use of stdlib dependencies in RBS * Mock additional call * Fix annotations * Update types in rspec undercover * Debug * Debug * Drop incorrect rbs collection use in spec * Update rubocop todo * Revert change * Fix RuboCop issue * Fix annotations * Fix annotations * RuboCop fix * Use "type arity" to guide signature combination * Update rubocop todo * Include return type arity in comparison * Add dodgy return type * Fix RuboCop issue * Add Ruby 4.0 jobs * Exclude another combo * Exclude another combo * Update rules to use report? * Fix merge * Fix merge * Drop dead code * Bump version to 0.59.0.dev.1 * Rename rule * Update RuboCop todo file * Update RuboCop todo file * Ratchet rubocop TODO file * Move to skip: * Mark spec as pending * Revert spec change * Drop old workaround * Fix merge * Fix typechecking issues * Revert doc * Fix spelling * Fix merge issue * Exclude the current gemspec from pins brought in from gem * Check pathname instead * Add sg-ignore * Avoid rbs pollution We were using the sig/shims directory for some internally helpful shims; unfortunately that exported them during gem installs, causing castwide#1144 * Test with RBS 4.0.0.dev.5 * Open up in gemspec * Fix missing spot * Typecheck using RBS prereleases * Move point of ignoring cached gems for gem projects * Fix issues resolving cgi escape functions * Be more careful marking things as stdlib * Reclassify rbs gem * Fix merge * Add sg-ignore * Fix merge * Remove outdated workaround * Fix @sg-ignore name * Restore workaround * Restore workaround * Merge branch 'flow_sensitive_typing_2_0' into 2025-01-06 * Fix method signature * Fix annotations * Add regression test and fix for issue found during future merge * Add regression test and fix for issue found during future merge * Fix merge * Fix merge * Fix merge * Fix merge * Use correct field for self type resolution Add a regression test and fix for self type resolution issue found on a future branch * Fix 'solargraph pin --references ClassName' private method call * Add error handling * Fix another location with another test case * Drop now-unneeded @sg-ignore * Don't log caching for each dependent library This causes duplicate logging on standard libraries, many of which are esoteric (e.g., "cgi-escaping"). The current method as of the 2025-01-06 branch would result in each stdlib library being cached individually. * Drop logging entirely * Fix some types based on future branch feedback * Provide Gem::Specification to outside interface * Provide Gem::Specification to outside interface * Use #to_spec * Provide Gem::Specification to outside interface * Fix typechecking error * Use consistent bundler versions * Fix type issue * Fix annotations based on future branch feedback * Add some @todos * Fix annotations * Fix annotations * Fix annotation * Add diff::lcs shim * Improve spec expectations * Add @sg-ignore * Fix rspec checks to run on all types of PRs * Fix merge * Fix merge * Fix merge
apiology
marked this pull request as ready for review
January 29, 2026 13:00
5 tasks
apiology
marked this pull request as draft
March 2, 2026 01:19
apiology
marked this pull request as ready for review
March 2, 2026 01:40
apiology
commented
Aug 1, 2026
| end | ||
|
|
||
| # @return [Hash{Array<String> => Array<Solargraph::Pin::Namespace, Solargraph::Pin::Constant>}] | ||
| # @return [Hash{Array, String => Array<Solargraph::Pin::Namespace, Solargraph::Pin::Constant>}] |
Contributor
Author
There was a problem hiding this comment.
This pattern of changes was brought in by a buggy version of YARD and the rubocop-yard autocorrect - please update both and revert the mistaken changes.
Contributor
Author
There was a problem hiding this comment.
Fixed in c789c8f. Root cause: these two YARD Hash tags were correct on both merge parents, but got mangled mid-merge by an errant RuboCop autocorrect run (YARD/CollectionStyle). Tested rubocop-yard 1.0.0 through 1.3.0 (latest) — the autocorrect bug reproduces on all of them (turns Hash{Array(String, Array<String>) => String, ...} into invalid Hash{Array, String, Array, String => String, ...}), so a version bump doesn't fix it. Reverted both tags to correct syntax and disabled YARD/CollectionStyle in .rubocop.yml so autocorrect can't silently re-mangle them.
Contributor
Author
There was a problem hiding this comment.
Found more instances of the same bug, fixed in f89c191: lib/solargraph/doc_map.rb (6 occurrences) and lib/solargraph/api_map/store.rb (2 occurrences) had the same Array(...) tuple parens stripped by the merge-time autocorrect. Confirmed all now match castwide/v0.59's original correct syntax exactly (diffed to verify zero remaining differences), and re-ran rubocop + affected specs clean.
Revert two YARD Hash tags in api_map/constants.rb that were mangled by a buggy rubocop-yard YARD/CollectionStyle autocorrect (confirmed still broken as of rubocop-yard 1.3.0, the latest release) back to their correct upstream syntax, and disable that cop since its autocorrect cannot be trusted for nested generic/paren Hash tag shapes. Remove the redundant "marks correct return type from RuboCop::Options.new" mapper spec (actually testing Open3.capture2e, stale name from an earlier perf-motivated swap) - it duplicated the Logger.new test above without adding distinct coverage.
Same buggy rubocop-yard autocorrect that hit api_map/constants.rb also
stripped the parens off Array(String, String) tuple-key Hash tags in
doc_map.rb (6 occurrences) and api_map/store.rb (2 occurrences),
turning them into invalid Hash{Array, String, String => ...} syntax.
Restored to match castwide/v0.59's original correct forms.
Contributor
Author
|
Closing in favor of #1237, which targets |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on v0.59 branch