fakefs / fakefs

A fake filesystem. Use it in your tests.
MIT License
1.04k stars 188 forks source link

Ruby3 Kernel.open() handling of kwargs fails after fakefs deactivation #490

Closed jpartlow closed 1 year ago

jpartlow commented 1 year ago

In Ruby3, after fakefs has hijacked Kernel.open, and then deactivated such that is once again delegating to the original Kernel.open, it can't handle passing of kwargs here in kernel.rb.

The issue is that in Ruby3 the positional and keyword args need to be separated. So for *args where args is something like ['a/file', {encoding: 'something'}], the kwargs hash ends up splatted as the second positional arg and you get a "fakefs/kernel.rb:22:in `initialize': no implicit conversion of Hash into String (TypeError)" error.

Example:

Given:

require 'fakefs/safe'

open('something', 'w') do |f|
  f.puts('testing')
end

open('something', encoding: 'ASCII-8BIT') do |f|
  puts f.read
end

FakeFS do
  puts 'hijacked methods'
end

open('something', encoding: 'ASCII-8BIT') do |f|
  puts f.read
end

you'll get:

$ ruby spec/test_spec.rb                                                                                                                
testing
hijacked methods
/home/jpartlow/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/fakefs-2.4.0/lib/fakefs/kernel.rb:22:in `initialize': no implicit conversion of Hash into String (TypeError)

          ::FakeFS::Kernel.captives[:original][name].call(*args, &block)
                                                          ^^^^^^^^^^^^^
        from /home/jpartlow/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/fakefs-2.4.0/lib/fakefs/kernel.rb:22:in `open'
        from /home/jpartlow/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/fakefs-2.4.0/lib/fakefs/kernel.rb:22:in `call'
        from /home/jpartlow/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/fakefs-2.4.0/lib/fakefs/kernel.rb:22:in `block (2 levels) in unhijack!'
grosser commented 1 year ago

ideally we'd use ... ... PR welcome (otherwise just *args, **kwargs, &block`)

jpartlow commented 1 year ago

I opened #491; looks like this could also be fixed with ::Kernel.send(:define_method, name.to_sym, ::FakeFS::Kernel.captives[:original][name].to_proc), but I don't know which approach is better.

grosser commented 1 year ago

looks good enough to me :)

On Fri, May 19, 2023 at 5:29 PM Josh Partlow @.***> wrote:

I opened #491 https://github.com/fakefs/fakefs/pull/491; looks like this could also be fixed with ::Kernel.send(:define_method, name.to_sym, ::FakeFS::Kernel.captives[:original][name].to_proc), but I don't know which approach is better.

— Reply to this email directly, view it on GitHub https://github.com/fakefs/fakefs/issues/490#issuecomment-1555396652, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAACYZYZNQ6JPUZPGRPESITXHAF77ANCNFSM6AAAAAAYIJSUF4 . You are receiving this because you commented.Message ID: @.***>