-
Notifications
You must be signed in to change notification settings - Fork 154
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.
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
Add task to verify indexing against Top 100 RubyGems #2330
Conversation
b97ac43
to
df48c61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I say let's run it on CI so it'll be maintained (or deleted when not used). I'm fine with downloading things on build if it's not too flaky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think enabling this in CI is a great idea. We can run this only on the latest Ruby version and only if the PR includes modifications to the ruby_indexer
directory or lockfile.
fa2418e
to
1e5f04d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If indexing succeeds, this is fine. However, if it fails, we will keep the copies of every gem still installed, even if only a single file out of the 100 gems have failed - which is why we need to ignore the tmp
directory for Sorbet.
You also get prints for every single file path, even the ones that succeed, which is not useful information for someone running the script.
Finally, if there is more than one error, you need to re-run the script because it will stop at the first failure.
How about we do something like this instead?
errors = Dir[File.join(directory, "**", "*.rb")].filter_map do |filepath|
print "."
code = File.read(filepath)
index.index_single(RubyIndexer::IndexablePath.new(nil, filepath), code)
nil
rescue => e
{ message: e.message, file: filepath }
end
# => Copy only the files with errors to some directory
# => group errors by filepath and then print them
# => include the error message as part of what is printed
2ae548c
to
3b8616e
Compare
3b8616e
to
1752518
Compare
Motivation
To help catch issues related to indexing, we can expand our indexing testing to cover a set of RubyGems
As per @kddnewton's suggestion, let's begin with just the top 100. We can later consider a periodic job which runs against every gem on RubyGems.org.
Implementation
Shamelessly copied from https://github.com/ruby/prism/blob/main/rakelib/lex.rake
Questions:
17.04s user 3.65s system 60% cpu 34.393 total
Automated Tests
None, since this is a test itself.
Manual Tests
Run with
bundle exec rake index:topgems
.