thoughtbot / factory_bot

A library for setting up Ruby objects as test data.
https://thoughtbot.com
MIT License
7.91k stars 2.6k forks source link

Improve experience with file uploads #1282

Open composerinteralia opened 5 years ago

composerinteralia commented 5 years ago

Related: https://github.com/thoughtbot/factory_bot/issues/385

On a current project we have a trait that looks like this:

    trait :with_profile_image do
      transient do
        profile_image_file { Rails.root.join("spec", "fixtures", "jpeg.jpg") }
      end

      after :build do |person, evaluator|
        file = evaluator.profile_image_file

        person.profile_image.attach(
          io: file.open,
          filename: file.basename.to_s,
        )
      end
    end

This seems overly complicated. Maybe we could have done something more like:

    trait :with_profile_image do
      profile_image do
        Rack::Test::UploadedFile.new('spec/fixtures/jpeg.jpg', 'image/jpeg')
      end
    end

But that also seems a little awkward to me, and something I am unlikely to remember.

I would like to at least see some documentation for the best way to handle file uploads in factory_bot, and maybe we could build something into factory_bot or factory_bot_rails to make this easier.

Maybe something like:

    trait :with_profile_image do
      profile_image { file_fixture("jpeg.jpg") }
    end

See https://github.com/rails/rails/blob/b9ca94caea2ca6a6cc09abaffaad67b447134079/activesupport/lib/active_support/testing/file_fixtures.rb#L24, https://github.com/rails/rails/blob/61c4be477706b721688e36a3168f86aabc625658/activestorage/test/test_helper.rb#L82-L84, and https://github.com/rack-test/rack-test/blob/a2f762d5abc2ead277dab51794d22083b72809ea/lib/rack/test/uploaded_file.rb for inspiration

composerinteralia commented 5 years ago

https://stackoverflow.com/questions/55141549/attatching-activestorage-files-in-factorybot https://blog.eq8.eu/til/factory-bot-trait-for-active-storange-has_attached.html

odlp commented 5 years ago

+1 to making this an easier experience.

Would this make sense as a feature / DSL-addon in Factory Bot Rails, rather than plain Factory Bot, since it's Rails specific functionality? Although we may then find it's less 'dicoverable' because I find myself reaching for the FB docs, not the FBR docs.

composerinteralia commented 5 years ago

I had the same thought about this maybe being in factory_bot_rails in https://github.com/thoughtbot/factory_bot/pull/1317#issuecomment-513345549, although I agree with you that might be less discoverable.

composerinteralia commented 4 years ago

Draft PRs https://github.com/thoughtbot/factory_bot/pull/1317 https://github.com/thoughtbot/factory_bot/pull/1327

composerinteralia commented 4 years ago

I played around with a branch that uses Rack::Test::UploadedFile: https://github.com/thoughtbot/factory_bot/commit/8d9bcda8fa22e410b09c38a42ca5dd153466533e.

Trying this out on a real project I ended up with this diff:

-def photo(photo_name)
-  Rack::Test::UploadedFile.new(Rails.root.join("spec", "fixtures", "photos", photo_name))
-end
+FactoryBot.file_fixture_path = Rails.root.join("spec", "fixtures")

 FactoryBot.define do
   factory :client_membership do
@@ -303,7 +301,7 @@ FactoryBot.define do
     end

     trait :with_image do
-      image { photo("ralph-bot.jpg") }
+      image { file_fixture("photos/ralph-bot.jpg") }
     end

In an ideal world I wouldn't have to specify the file_fixture_path. But given that it is common to use factory_bot in the Rails console, where ActiveSupport::TestCase.file_fixture_path would not available, this may be the best option.

If we go this route it probably makes sense to allow setting the file_fixture_path in the DSL as well, like:


FactoryBot.define do
  file_fixture_path { "path/to/fixtures" }
end
dorianmariecom commented 3 years ago

still an issue, would be nice to have file_fixture in factories ❤️

doutatsu commented 2 years ago

Bump this

agrobbin commented 2 years ago

For those who are still looking for a potential solution to this problem, and are also using Rails and RSpec, I took this approach:

module FactoryBotFileFixtureHelpers
  extend ActiveSupport::Concern

  include ActiveSupport::Testing::FileFixtures
  include ActionDispatch::TestProcess::FixtureFile

  included do
    self.file_fixture_path = RSpec.configuration.file_fixture_path
  end
end

FactoryBot::Evaluator.include FactoryBotFileFixtureHelpers
FactoryBot::SyntaxRunner.include FactoryBotFileFixtureHelpers

This exposes both file_fixture and fixture_file_upload. It may not work for everyone's situation, but hopefully it helps someone who ends up here!

Edit (2022-04-19): Forgot about FactoryBot::SyntaxRunner for usage in callbacks!

TylerRick commented 2 years ago

Can we please add out-of-the-box support for file_fixture? The need to be able to use it from factories seems pretty common.

I stumbled upon what seems to be an even simpler solution — thanks to rspec-rails/lib/rspec/rails/file_fixture_support.rb having already done the work of setting file_fixture_path for us:

# spec/support/file_fixtures.rb
FactoryBot::SyntaxRunner.class_eval do
  include RSpec::Rails::FileFixtureSupport
end

Then I can use file_fixture in my factories like this...

    transient { avatar_file { nil } }
    avatar_file { file_fixture('sample.jpg') }
    after :build do |record, e|
      record.avatar.attach(
        io:       e.avatar_file.open,
        filename: e.avatar_file.basename.to_s,
      )
    end
dorianmariecom commented 2 years ago

I feel like this is a rails/rails issue, we should be able to attach a file with something like User.create(avatar: File.open("image.jpg") or User.create(avatar: file_fixture("image.jpg")

For File.open, it returns a File:

File.basename(File.open("app/../image.jpg").path) # => "image.jpg"

So it's doable, I will try to implement it on rails/rails (hopefully it gets accepted)

dorianmariecom commented 2 years ago

I opened a pull request on rails/rails https://github.com/rails/rails/pull/45606

TylerRick commented 1 year ago

It looks like after https://github.com/rails/rails/pull/45606, it will simplify my example above to just this:

    avatar { file_fixture('sample.jpg') }

That will be great!

Though if I'm not mistaken, it will still require a bit of extra work to configure it so you can actually use file_fixture from within a factory. That's really the part that I was hoping we could improve. Any chance this gem could take on the responsibility of making file_fixture available (automatically) so the various hackery options above aren't needed?

seanpdoyle commented 11 months ago

https://github.com/rails/rails/pull/45606 has been merged, and will likely be part of the 7.1 release.

Along with that change, what else can be done?

dorianmariecom commented 11 months ago

hi @seanpdoyle, it seems like file_fixture is not defined in factory bot

NoMethodError: undefined method `file_fixture' for #<FactoryBot::SyntaxRunner:0x000000010a69bd78>
seanpdoyle commented 11 months ago

@dorianmariefr I've opened https://github.com/thoughtbot/factory_bot_rails/pull/427 to attempt to add support for file_fixture to Factories.