dry-rb / dry-rails

The official dry-rb railtie
https://dry-rb.org/gems/dry-rails
MIT License
270 stars 27 forks source link

Key-based Import doesn't get stubbed #19

Open mgpnd opened 4 years ago

mgpnd commented 4 years ago

Describe the bug

When a dependency is imported via key-based argument, stubbing doesn't affect it's resolving.

I've made a sample app that reproduces the issue: https://github.com/mgpnd/import_example

Here's the most important snippets from the app:

# Operation that will be imported and stubbed
module Example
  class Show
    def call
      self.class.name
    end
  end
end
class ExampleController < ApplicationController
  include ImportExample::Import[
    show_example: "example.show"
  ]

  def index
    @auto_imported = show_example.()
    @manually_resolved = ImportExample::Container["example.show"].()
  end
end

/spec/controllers/example_controler_spec.rb:

RSpec.describe ExampleController, type: :controller do
  describe '#index' do
    subject { get :index, params: { format: 'text/hmtl' } }

    ...

    before do
      # Stubbing is enabled somewhere else so this part works properly
      ImportExample::Container.stub('example.show', -> { 'operation stub' })
    end

    # Fails, @auto_imported == "Example::Show"
    it 'calls auto imported stub' do
      subject
      expect(assigns(:auto_imported)).to eq('operation stub')
    end

    # Passes, @manually_resolved == "operation stub"
    it 'calls manually resolved stub' do
      subject
      expect(assigns(:manually_resolved)).to eq('operation stub')
    end
  end
end

To Reproduce

  1. Define a dependency via auto_inject Import with key-based argument
  2. Stub the dependency in any test

Expected behavior

The dependency resolved to provided stub.

Your environment

I've tried to reproduce the issue with plain dry-auto_inject and with basic dry-system, but it doesn't appear there, in both gems importing works properly so I'm reporting it here.

solnic commented 4 years ago

Thank you for reporting this and especially for checking it in a plain dry-system setup. I'll look into this, it's very likely that test mode in Rails will require some special care :/

dsalamau commented 4 years ago

Update

I'd like to add that when a dependency is imported via array import stubbing doesn't affect it's resolving too.

I've created a PR to this sample app that reproduces the key based args issue: https://github.com/mgpnd/import_example/pull/1/files

Please, take care about snippets below:

# Array import operation example
class ArrayImportExampleController < ApplicationController
  include ImportExample::Import[
    "example.show"
  ]

  def index
    @auto_imported = show.()
    @manually_resolved = ImportExample::Container["example.show"].()
  end
end
# Specs
require 'rails_helper'

RSpec.describe ArrayImportExampleController, type: :controller do
  describe '#index' do
    subject { get :index, params: { format: 'text/hmtl' } }

    it 'calls auto imported operation' do
      subject
      expect(assigns(:auto_imported)).to eq('Example::Show')
    end

    it 'calls manually resolved operation' do
      subject
      expect(assigns(:manually_resolved)).to eq('Example::Show')
    end

    context "when operations is stubbed" do
      before do
        ImportExample::Container.stub('example.show', -> { 'operation stub' })
      end

      it 'calls auto imported stub' do
        subject
        expect(assigns(:auto_imported)).to eq('operation stub')
      end

      it 'calls manually resolved stub' do
        subject
        expect(assigns(:manually_resolved)).to eq('operation stub')
      end
    end
  end
end
# Result
ArrayImportExampleController#index when operations is stubbed calls auto imported stub
     Failure/Error: expect(assigns(:auto_imported)).to eq('operation stub')

       expected: "operation stub"
            got: "Example::Show"
stefanrendevski commented 4 years ago

@dsalamau @mgpnd I was looking into this for my own project. I am not exactly certain where the problem is, but the following setup allows for correct stubs:

# spec/controllers/some_controller_spec.rb
RSpec.describe SomeController do
  before do
    setup_stubs
    @controller = described_class.new
  end

  let(:setup_stubs) do
    ImportExample::Container.stub('example.show', -> { 'operation stub' })
  end
end

This leads me to believe that an instance of a controller is created before the test setup in a spec is executed, and it is that instance that is used when you submit a request.

solnic commented 4 years ago

@stefanrendevski you definitely want to set up stubs in a before hook and the rspec integration (that we don't have yet) should make sure that container stubbing is setup/teared down in the right moment.

ndan commented 3 years ago

It works for me in dry-system-rails, but not in dry-rails