paulelliott / fabrication

This project has moved to GitLab! Please check there for the latest updates.
https://gitlab.com/fabrication-gem/fabrication
MIT License
998 stars 97 forks source link

Remove global Fabrication.manager reference in Defintion #313

Open bradgessler opened 5 years ago

bradgessler commented 5 years ago

Opening this PR to see if you're open to me removing more global references inside of Fabrication.

I have a weird use case where I need to use Fabrication both for testing/development in rails, but I also need a completely separate object generator in production that will live in app/factories. I'm hoping to isolate all of the global/singleton state into one class that lives outside of the Manager for the "normal" test/development use case.

paulelliott commented 5 years ago

Sure, I am open to talking about it. Can you explain in more detail why exactly the singleton here is an issue for your development and test case?

bradgessler commented 5 years ago

Yeah!

I actually need to maintain two completely different sets of factories. The obvious set is for testing and development environments, which ‘Fabricator(:blah)’ handles well.

The problem is I don’t want the factories that will be used in my application code to have access to the test/development factories, so I need to create another instance of the manager to handle those factories.

From what I’ve seen so far in the code, most of the work involved in getting this out of a global dependency is the commits I’ve already done, then figuring out how to extract the file loader into another class that the manager can delegate to, and finally moving the ‘Factory(:blah)’ methods into a class, then delegating to an instance of that from the ‘Factory’ class/module

paulelliott commented 5 years ago

Would it not work for you to just override the fabricator path for production? Something like this:

Fabrication.configure do |config|
  config.fabricator_path = Rails.env.production? ? 'spec/fabricators' : 'app/fabricators'
end
bradgessler commented 5 years ago

It wouldn’t in this case because I need to test the models that user the factories from the production code.

On May 23, 2019 at 14:44, <Paul Elliott (mailto:notifications@github.com)> wrote:

Would it not work for you to just override the fabricator path for production? Something like this:

Fabrication.configure do |config| config.fabricator_path = Rails.env.production? ? 'spec/fabricators' : 'app/fabricators' end

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub (https://github.com/paulelliott/fabrication/pull/313?email_source=notifications&email_token=AAABEFDLK7D3UWQNIVWUGJTPW4F5XA5CNFSM4HO2N6LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDSLIY#issuecomment-495396259), or mute the thread (https://github.com/notifications/unsubscribe-auth/AAABEFFSH3OE3SCAABRBUHDPW4F5XANCNFSM4HO2N6LA).

paulelliott commented 5 years ago

You could specify multiple paths. Like a base fabricator path then switch on a second folder that has prod and no prod. Would that work?

I'm not opposed to these changes but if there's a simple way to do it that's less risky I feel like it's worth exploring.

On Thu, May 23, 2019, 6:21 PM Brad Gessler notifications@github.com wrote:

It wouldn’t in this case because I need to test the models that user the factories from the production code.

On May 23, 2019 at 14:44, <Paul Elliott (mailto:notifications@github.com)> wrote:

Would it not work for you to just override the fabricator path for production? Something like this:

Fabrication.configure do |config| config.fabricator_path = Rails.env.production? ? 'spec/fabricators' : 'app/fabricators' end

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub ( https://github.com/paulelliott/fabrication/pull/313?email_source=notifications&email_token=AAABEFDLK7D3UWQNIVWUGJTPW4F5XA5CNFSM4HO2N6LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDSLIY#issuecomment-495396259), or mute the thread ( https://github.com/notifications/unsubscribe-auth/AAABEFFSH3OE3SCAABRBUHDPW4F5XANCNFSM4HO2N6LA ).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/paulelliott/fabrication/pull/313?email_source=notifications&email_token=AAAOJ5EGLE5EFVRV35TQNV3PW4KG5A5CNFSM4HO2N6LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDUW3Q#issuecomment-495405934, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAOJ5DX53RSPNAJYMLBBLDPW4KG5ANCNFSM4HO2N6LA .