Remove global Fabrication.manager reference in Defintion #313
Conversation
The Manager can inject this into the Definition class since there's no apparent public callers outside of the Fabrication gem that would use this class.
|
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? |
|
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 |
|
Would it not work for you to just override the fabricator path for production? Something like this: |
|
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 ***@***.***)> 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 (#313?email_source=notifications&email_token=AAABEFDLK7D3UWQNIVWUGJTPW4F5XA5CNFSM4HO2N6LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDSLIY#issuecomment-495396259), or mute the thread (https://github.com/notifications/unsubscribe-auth/AAABEFFSH3OE3SCAABRBUHDPW4F5XANCNFSM4HO2N6LA).
|
|
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 ***@***.***> 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 ***@***.***)>
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 (
#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
<#313?email_source=notifications&email_token=AAAOJ5EGLE5EFVRV35TQNV3PW4KG5A5CNFSM4HO2N6LKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWDUW3Q#issuecomment-495405934>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAOJ5DX53RSPNAJYMLBBLDPW4KG5ANCNFSM4HO2N6LA>
.
|
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.