Add the ability to create Test::UploadedFile instances without the file system

[amilligan]

Uploaded files behave as IO streams, so it should be reasonable to instantiate a fake one from an IO stream. These changes don't change the existing interface to the #initialize method, but add the ability to do something like this:

io = StringIO.new('Some content')
upload = Rack::Test::UploadedFile.new(io, original_filename: 'wibble.txt')
...

A lot of tests for content uploads don't need a lot of complex content; this should allow for test suites without a bunch of tiny test files cluttering the project or spec directories.

[dennissivia]

Thanks for the contribution. I really like the idea, but I am not to sure about all details of the implementation. Especially the conditional switch on respond_to to then handle IO object and paths differently.
However I don't want to to reject anything for that reason. What do you think @perlun and @junaruga

[junaruga]

@scepticulous I like the idea too, though I have not checked the actual source code yet.
Thanks for the contribution.

[perlun]

@amilligan This has now conflicts with the current master code. Are you still around, and if so, could you try to rebase this on top of the current tree?

[amilligan]

Okay, I fixed the conflicts; nothing too concerning.

Regarding an earlier concern: I don't like using #respond_to? in almost any circumstances, but in this case the existing initializer interface expects a filename as a string, which is not easily reconciled with also accepting an abstract IO stream. Ideally, the initializer would accept only an IO stream, but that would break the existing interface.

I can think of any number of ways to work around this, but #respond_to? seems to be the most generally accepted in the Ruby community. I'm open to suggestions, if you have preferences.

[perlun]

@amilligan this now has conflicts again. 😄 If you rebase one more time, I promise we'll try to give it a look.

[amilligan]

Resolved again; ready for a look.

[perlun]

One question that I think should be resolved before merge, but in general - thanks! A valuable contribution. @scepticulous and @junaruga, feel free to chime in as well.

[perlun]

https://ruby-doc.org/core-2.4.1/Object.html#method-i-respond_to_missing-3F says:

DO NOT USE THIS DIRECTLY.

Is there a way to implement this in another way?

[junaruga]

Yeah, sure.

[amilligan]

@perlun I'm slightly confused. You're asking about the choice to implement #respond_to_missing? as opposed to #respond_to??

As of ruby 1.9 implementing the former is the invariably preferred mechanism. The 'DO NOT USE THIS DIRECTLY' directive refers to calling the method, which I don't believe is done anywhere in the project.

[perlun]

@perlun I'm slightly confused. You're asking about the choice to implement #respond_to_missing? as opposed to #respond_to??

As of ruby 1.9 implementing the former is the invariably preferred mechanism. The 'DO NOT USE THIS DIRECTLY' directive refers to calling the method, which I don't believe is done anywhere in the project.

I stand corrected. 🙈 Sorry for that @amilligan! This blog post explains some more of the details.

I am fine with this now. Will approve and merge. Thanks for the effort, and sorry about the delay!

[tagliala]

Hi,

This change is breaking an use case in specs with Paperclip + FactoryBot.

The suggested method documented here: https://github.com/thoughtbot/paperclip#testing

FactoryGirl.define do
  factory :user do
    avatar { File.new("#{Rails.root}/spec/support/fixtures/image.jpg") }
  end
end

does not work anymore:

     NoMethodError:
       undefined method `size' for nil:NilClass
     # ~/dev/rack-test/lib/rack/test/uploaded_file.rb:38:in `public_send'
     # ~/dev/rack-test/lib/rack/test/uploaded_file.rb:38:in `method_missing'
     # ~/dev/rack-test/lib/rack/test/utils.rb:136:in `build_file_part'
     # ~/dev/rack-test/lib/rack/test/utils.rb:101:in `block in get_parts'
     # ~/dev/rack-test/lib/rack/test/utils.rb:92:in `each'
     # ~/dev/rack-test/lib/rack/test/utils.rb:92:in `map'
     # ~/dev/rack-test/lib/rack/test/utils.rb:92:in `get_parts'
     # ~/dev/rack-test/lib/rack/test/utils.rb:87:in `build_parts'
     # ~/dev/rack-test/lib/rack/test/utils.rb:77:in `build_multipart'
     # ~/dev/rack-test/lib/rack/test.rb:231:in `env_for'
     # ~/dev/rack-test/lib/rack/test.rb:128:in `custom_request'
     # ~/dev/rack-test/lib/rack/test.rb:66:in `post'

Is there any suggested way of doing this with rack-test 0.7.1? Should I report this to Paperclip?

I've tried several things but I'm unable to make it work.

TIA

[007lva]

Hi,

I have the same problem with 0.7.1: rspec/rspec-rails#1915

[perlun]

@tagliala @007lva Sorry for this; this is an unintended regression because of this change. For now, downgrade your project to 0.7.0 to work around the issue.

If either one of you could provide a PR with a failing test case to the rack-test repo, that would be 👍 👍 👍. We will then try to fix this as soon as possible, and get a fixed gem version released.

I will lock this conversation now, since it's too hard to find discussions going on in "merged pull requests"; we need it in a new issue or PR for it to be visible to the audience at large.

[perlun]

The issue @tagliala reported is now here: #214