Refactor Bundler tests to invoke RubyGems API directly#9195
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Bundler tests to invoke the RubyGems API directly instead of spawning subshells, improving test suite performance. The primary change removes the gem_command helper method and replaces all its usages with direct API calls to RubyGems classes.
- Removes the
gem_commandhelper that spawned subshells for gem operations - Refactors
install_gemto useGem::Installer.atAPI directly - Adds
uninstall_gemandinstalled_gems_listhelper methods that call RubyGems APIs
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bundler/spec/support/helpers.rb | Removes gem_command method; refactors install_gem to use Gem::Installer.at; adds new uninstall_gem and installed_gems_list helper methods for direct API calls |
| bundler/spec/support/builders.rb | Replaces gem build subshell command with direct Gem::Package.build API call |
| bundler/spec/install/gems/compact_index_spec.rb | Updates test to use new uninstall_gem helper instead of gem_command |
| bundler/spec/commands/clean_spec.rb | Updates tests to use new installed_gems_list helper instead of gem_command :list |
| bundler/spec/commands/check_spec.rb | Updates test to use new uninstall_gem helper instead of gem_command |
| bundler/spec/bundler/gem_helper_spec.rb | Updates test to use new installed_gems_list helper instead of gem_command :list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+359
to
+376
| # Temporarily set GEM_HOME for the command | ||
| old_gem_home = ENV["GEM_HOME"] | ||
| ENV["GEM_HOME"] = gem_home | ||
| Gem.clear_paths | ||
|
|
||
| begin | ||
| require "rubygems/commands/list_command" | ||
|
|
||
| # Capture output from the list command | ||
| output_io = StringIO.new | ||
| cmd = Gem::Commands::ListCommand.new | ||
| cmd.ui = Gem::StreamUI.new(StringIO.new, output_io, StringIO.new, false) | ||
| cmd.invoke | ||
| output = output_io.string.strip | ||
| ensure | ||
| ENV["GEM_HOME"] = old_gem_home | ||
| Gem.clear_paths | ||
| end |
There was a problem hiding this comment.
The installed_gems_list method manually saves and restores ENV["GEM_HOME"], but this codebase has an established pattern for temporary environment variable modifications using the without_env_side_effects helper (see lines 393-415). Using this helper would be more consistent with the codebase's patterns and provide better safety guarantees, as it ensures all ENV changes are properly restored even if an exception occurs during the begin block.
… and listing installed gems
hsbt
force-pushed
the
colby/remove-test-subshelling
branch
from
eb62a3c to
185964f
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
hsbt
force-pushed
the
colby/remove-test-subshelling
branch
from
0f048fc to
65f6b69
Compare
When install_gem runs Gem::Installer in-process, BUNDLER_SPEC_PLATFORM is not applied because hax.rb only runs in subprocesses. Add with_simulated_platform helper that mirrors hax.rb's platform override for in-process operations, and restore psych in standalone extension test's base_system_gems. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
hsbt
force-pushed
the
colby/remove-test-subshelling
branch
2 times, most recently
from
007aebf to
b9c831a
Compare
With load_relative enabled (ruby-core CI), Gem::Installer generates wrapper scripts with a bash prolog (#!/bin/sh) when wrappers: true is set. Bundler's directly_loadable? only recognizes Ruby shebangs, so these shell wrappers cause "command not found" errors in nested bundle exec scenarios. Adding env_shebang: true forces #!/usr/bin/env ruby shebang, which directly_loadable? recognizes, fixing 6 test failures in ruby-core CI while keeping wrappers to avoid double-loading issues with symlinks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gem::Uninstaller with install_dir uses File.realpath for @gem_home but not for the specification_record path lookup, causing path mismatches on some CI environments. Instead, temporarily set GEM_HOME and call Gem.clear_paths to mimic the old subprocess-based gem_command behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
hsbt
approved these changes
Mar 18, 2026
Member
|
@colby-swandale Finally, I fixed CI failures with your changes. I prefer your proposal over shell spawn. I'm going to merge this. |
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
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.
Some Bundler tests currently spawn a subshell to invoke RubyGems. We're moving away from this pattern to speed up the test suite.
This PR refactors those tests to call the RubyGems API directly instead.
The ultimate prize is replacing the
bundlerandbundlehelpers in the test suite entirely, but this serves as a good stepping stone towards that.