Project

General

Profile

Actions

Misc #22296

open

Ruby::Box: method stubs on core classes are invisible to internal builtins

Misc #22296: Ruby::Box: method stubs on core classes are invisible to internal builtins

Added by hsbt (Hiroshi SHIBATA) 22 days ago. Updated 19 days ago.

Status:
Open
Assignee:
-

Description

Under RUBY_BOX=1, redefining a core class method affects only the box where the definition happens. Code loaded with require runs in the main box and observes the redefinition, but methods implemented as internal builtins do not. This breaks the way test frameworks install doubles, and the breakage is partial rather than total, which makes it hard to diagnose.

require "pathname"
require "rspec/mocks/standalone"

allow(File).to receive(:expand_path).and_return("/stubbed")

puts File.expand_path("x")            # => "/stubbed" in both modes
puts Pathname.new("x").expand_path    # => "/stubbed" normally, real path under RUBY_BOX=1

The same divergence appears without any gem by defining def File.expand_path(*) = "/stubbed" directly. A pure Ruby library that I require and that calls File.expand_path does observe the stub, so this is not general box isolation of stdlib. It is specific to builtins, and Pathname#expand_path reports <internal:pathname_builtin> as its source location. Defining the method through Ruby::Box.root.eval does not help either, because the builtin still does not see it.

I hit this in the RubyGems test suite while making it green under RUBY_BOX=1. A blanket File.expand_path stub was honored by our own code but bypassed by Pathname, so an unrelated path resolved for real and the example failed far from the stub. I worked around it in ruby/rubygems#9826 by scoping the stub to the exact arguments under test.

My question is about the intended design rather than this one case. Blanket stubs on File, Time and other core classes are extremely common in existing test suites. Do we treat this as a breaking change and ask test authors to write box-aware stubs, or should Box make such redefinitions visible to builtins? If the former, the behavior needs to be documented before Box becomes the default, and rspec-mocks and similar libraries probably need a way to detect it. If the latter, we need to decide how far the visibility extends without giving up the isolation Box is meant to provide.


Related issues 1 (1 open — 0 closed)

Related to Ruby - Misc #22275: Ruby::Box support plan for RubyGems and BundlerOpenActions

Updated by hsbt (Hiroshi SHIBATA) 22 days ago Actions #1

  • Related to Misc #22275: Ruby::Box support plan for RubyGems and Bundler added

Updated by hsbt (Hiroshi SHIBATA) 19 days ago Actions #2

The workaround I used in ruby/rubygems#9826 does not make the stub visible to the builtin. It removes the example's dependence on that visibility.

The spec had

allow(File).to receive(:expand_path).and_return("#{install_path}/bundler/setup")

and became

allow(File).to receive(:expand_path).and_call_original
allow(File).to receive(:expand_path).with("setup", anything).and_return("#{install_path}/bundler/setup")

The reason the first form ever worked is that it replaced every result with one fixed string. The code under test resolves several unrelated paths besides the one being asserted, and they were all the same fiction, so nothing could disagree. A box breaks exactly that: the calls made from Ruby code keep seeing the stub, the ones that go through a builtin go back to the real filesystem, and the run is left half faked. The example then fails somewhere far from the stub, on a path it was never about.

and_call_original with a narrowed with confines the fiction to the single call the example asserts on. Everything else returns a real value, in both modes, whether or not the builtin can see stubs. There is no longer a second version of the truth for the box to diverge from.

The other way out is to stub the builtin as well, which keeps the blanket intent:

allow(File).to receive(:expand_path).and_return("/stubbed")
allow_any_instance_of(Pathname).to receive(:expand_path).and_return(Pathname.new("/stubbed"))

I verified this shape on the script in the description: with the second line the box-enabled run prints /stubbed for both. It works because dispatch happens in the caller's box, and only the lookup inside the builtin falls back to master. The cost is that the author has to enumerate every builtin that fans out from the method being stubbed, which is not something a test suite can know in advance.

Actions

Also available in: PDF Atom