Stubbing a Provider (formerly known as Bootable component)

Support dry-system
1

Hello,

I have a quite simple use case : I’m using a s3 provider in order to access… well s3 .

It looks like this :

App::Container.register_provider(:s3) do
  prepare do
    require 'aws-sdk-s3'
  end

  start do
    use :settings
    settings = target[:settings]

    s3_client = Aws::S3::Resource.new(
      region: settings[:aws_region]
    )

    register(:s3, s3_client)
  end
end

Now in my tests, I definitely do not want to hit any real buckets, I want to use a stub for this component.

Therefore I’m using this :

  before do
    fake_s3_client = double("fake s3 client").as_null_object
    App::Container.stub(:s3, fake_s3_client)
  end

Unfortunately, App::Container.stub(:s3,...) calls App::Container.key?(:s3) which calls App::Container.resolve(:s3), which eventually boots my component which is exactly what I did not want to happen.

I’m okay(-ish) with the idea that the prepare lifecycle of the component is called but definitely not the start lifecycle.

Obviously I could create the whole chain of dependencies of my subject under test by hand and introduce the stubs where I need them , but that would also be missing the point of the integration test I’m trying to write.

Am I missing something ?

2

I think it is highly related to this post : Avoid doing http calls in test env

3

Why not?

In general, what I do is disable all http requests with WebMock.disable_net_connect! and then allow any http clients in the system to initialize, which creates the keys that I can then stub in the spec.

around :each do |example|
  App::Container.stub(:s3, fake_s3_client) { example.run }
end

Alternatively, you could mock out the client in your test subject

subject { described_class.new(s3: fake_client) }

but that’s probably not practical in some cases.

Finally, since providers can be optionally switched off, I suppose you could exclude it from the test environment and register the key in your spec config.

App::Container.register_provider :s3, if: App::Container.env != :test do
  # etc
end

I would be less inclined to do that, because I actually want the real client when writing an integration test, so I can webmock the requests.

4

Thanks for your answer

Why not?

Because it is absolutey counter intuitive. When I call Container.stub(...), my intent is to stub what the container does when someone calls Container[...].

Consider the parallel with a method stub : When I stub the method, I do not expect the actual method to run but change what it returns, I want to actually prevent the method to run and instead return another value. If I use this example to its maximum potential, I think I would actually enjoy having a Container.stub(key,&blk) where blk could the the prepare + start code to create the stubbed value in a very similar way to how stubbing a method works.

Maybe we could have an explicit Container.stub(key, boot_original: true) for when we actually want the bootable component to boot but I would bet that this is not the usage most people want.

Using Webmock for integration testing, definitely makes sense. The question with integration testing is always “how far goes the scope”, I like to have a scope covers only my own code and does not cover external libraries and standard lib. This is why I use the container to provide me things like Open3, CSV, AWS::S3::Resource, … Typically I do not consider that a developer but know anything about what exactly the s3_client does on the network to use it.
Also it makes my specs more stable, if at some point the gem is updated and uses the new version of the AWS API, my tests won’t have to change. If I used webmock, I would have to change my tests even though nothing in my own code justifies it.

Alternatively, you could mock out the client in your test subject

That’s what I ended up doing. Effectively changing my integration spec in a unit spec.
If I want to have some non-stubbed dependencies, I’ll simply instantiate them manually. This has the big downside of making me not test the integration with the container and auto injection.

Finally, since providers can be optionally switched off, I suppose you could exclude it from the test environment and register the key in your spec config.

I did not know about this but I’m not inclined to do so :

  • As you said, I may want to have the real client in some tests
  • I hate when my code has environment-related conditions. I have an environment specific “loader” ( spec_helper), that is the piece of code responsible for setting up everything that is test oriented. If I start spreading environment specific condition in my code, I’ll live a maintenance hell and a communication hell with my team.

Thank you again for your answer. I understand that what I wanted is not doable but you provided interesting workaround. I hope my explanations can be useful for anyone working on the gem in the future but as they say “beggars are not choosers” :slight_smile:

5

Note that stubbing a method that does not exist is considered a failure. The original implementation needs to be there first, so that you can stub it. That is how container stubs work.

6

Disclaimer : I do not want to sound like I have bad faith. I’m replying mostly for the sake of finding what is the “correct” intuition when it comes to stub. Maybe the final conclusion is that stub are not what I think, or maybe it’s simply the wrong word to be used in a container’s context.

The original implementation of a stubbed method does not have to exist. Only the signature has to exist. And the implementation is actually not called (by default) when you stub a method.

Furthermore, you are mentioning stubs of typed objects. When I TDD/BDD, I very often stub my dependencies as double("fake foobar").as_null_object and stub whatever I want on it in order do discover what is the actual public interface of my dependency.

If I try to draw the parallel with dry-system. I think it would be ok to check that the provider is registered ( Application.register_provider(:foobar) ) but not run its prepare nor its start phase.

I actually find it a little bit confusing to have to specify both the provider name and the component name. In my opinion it would make sense to force them to be the same and only specify the provider name. Then the start phase would simply be allowed to call register(my_object) which would automatically be registered under the name specified during the register_provider.

If a provider wants to register several components, I would have it register a hash or a struct with the different components. Very much like how the settings works. It register all settings even though we almost never need to inject all settings in a component.

@solnic , could you maybe shed some light on your thoughts about this ?

1 Like
7

I’ll make sure I take a look at this before we bring dry-container and dry-system to 1.0 (which will happen sometime in the next few months). Thanks for all the context here, @systho, it’s really helpful.

2 Likes
8

These are totally understandable questions to ask, @systho. I ran into this problem myself, which caused me to look at the underlying implementation to understand it better. The usage of container stubs is under-documented right now, certainly.

In general, I don’t agree that you can make the distinction between a method’s signature and its implementation. I suppose the existence of RBS means that perhaps Ruby is moving in that direction, but it is still the case that a method is either there or not there.

Now, the specifics of Ruby methods actually don’t matter because we’re really talking about container addresses and maybe that muddies the waters. It’s even more confusing because we generally access containers as if they’re Hashes, and that’s not really accurate either.

I will admit that I was originally too focused on the specific meaning of these words in the context of RSpec, and while that is a reasonable assumption in many cases, it’s certainly possible to correctly use these words differently. A quick re-read of Martin Fowler’s definition shows that yes, what you are describing could be called stubbing, and the confusion I had is fairly common. Mea culpa

While container stubs do not behave this way now, I can see the argument for it.

I think the current behavior is the safest, because what you don’t want to do is stub out interfaces in your test code that never actually exist in production. Requiring a preexisting container key prevents this possibility.

It should be possible, however, to change stubs to override the real implementation of a provider in a lazy way, that would only take effect if the key is called upon. I think that would require a new interface for getting the original value somehow, though.

1 Like
9

Thanks for having taken the time to reply in such a peaceful way. I’m not used to rational discussions on Internet; it’s refreshing.

I think we have reached the end of the reasonable scope of this ticket.
You gave me the hints I needed two message ago :slight_smile:

If you want me to participate in any way in future design or stubbing, I would be happy to provide additional thoughts and examples. But I think there are plenty of people with ideas and very few with the time to implement them, I think the ones who do the job are entitled to do it the way they want :slight_smile:

Thanks @alassek for your messages and your time
Thanks @timriley for having taken the time to read all this

1 Like
10

Hey @systho - apologies for late reply, gmail decided to treat my discourse notifications as spam and I missed this!

Anyhow, I’d like to clarify a couple of things. The stubbing feature is in a very basic form right now. Since day 1 I’ve had quite ambitious plans for stubbing deps when you use dry-system. The final form of this feature should actually come with a built-in support for proper contract testing. This means that if you’re running your spec suite, and you stub :s3 dependency, the system should trace which methods are being used on the stub dependency while executing specs and then it should verify if these methods are actually covered by s3’s specs. With such approach we’d “push” people towards using proper abstractions, in your case it would be better to have a simple wrapper around s3 client so that the API surface that you depend on is going to be limited. This way you could actually ensure that stubbing or mocking in specs won’t be giving you false positives.

Keeping that in mind, I also want to confirm that stubbing a dependency should indeed not invoke start lifecycle step, because it’s not needed. There is a caveat here though - like I mentioned, you really want to have specs for the real thing too. This means that you may want to start a dep in order to execute its tests. If starting a dep may result in stuff like connecting to external resources, and you want to avoid that in tests, then you simply need to guard against it in your provider files. We could provide convenient APIs for that.

I actually think we should finish stubbing feature for dry-system 1.0.0. Contract testing can be added later on, but Container.stub method and its behavior should be revisited prior 1.0.0.

Thanks for bringing this up!

Oh and:

I was very pleased to read this, thanks for expressing your appreciation :heart:

11

I’m super excited to see what the 1.0.0 will look like , and also what the future will hold !
I strongly believe that languages and frameworks should not only facilitate the life of devs through providing well designed tools and abstractions but also encourage clean code organisation and adoption of good principles. I can’t wait to see what you’ll do about proper contract testing.

Keep up the good works peepz, we love what you do !

1 Like
12

@systho for your particular use case, and you may know this but AWS does have a pretty extensive mocking lib (and we are reaching for it more and more frequently rather than providing our own behavior), and we use Container and AutoInject fairly regularly in our app, but do not use them as bootable components, rather register them as injected dependencies, newing a client up each time. This seems to work out well and I’m not sure we would get a ton by booting a client at init vs providing one from the container as necessary:

a sort of the high points version would look like this:

module Documents
  class Container
    register "s3_client" { Aws::S3::Client.new }
  end

  Import = Dry::AutoInject(Container)

  class GetFile
    include Import[client: "s3_client"]
    include Dry::Monads[:try]

    def call(file_arn)
      arn, bucket, key = parse_arn(file_arn)
      Try(Some::Aws::Error) { client.get_object(bucket: bucket, key: key).body }.to_result
    end
    
end

# in test
describe Documents::GetFile do
  subject(:get_file) { described_class.new(client: client }

  let(:client) { Aws::S3::Client.new(stub_responses: true) }

  it "does the thing" do
    client.stub_response(:get_object, body: File.read("path to fixture").to_s)
    subject.call(some_arn) in Success(file)
  end
  
  it "doesn't do the thing" do
    client.stub_response(:get_object, 'NoSuchKey')
    subject.call(some_arn) in Failure(reason)
  end
end

For places where we do not have instance access, as others have said, we stub the container itself and provide the dependencies to all instances pulling from the container (more useful for integration level testing).

This is our strategy, and it works pretty darn well. Hope it helps!

1 Like
13

Thank you @jedschneider !
This is super useful !

I knew AWS had a stubbing library but having such a specific example about how to use it really helps me understand how I can articulate it.

This will definitely help me to have tests which do not need more lib knowledge than my code. If I use the gem in my code, I’ll use the gem stubbing abilities, and if I issue http requests, I’ll use webmock.

1 Like
14

This probably ventures a bit abroad and OT, but on the topic of webmock, another handy aws-sdk feature is you can do full HTTP wire traces by providing to the client the option, eg: Aws::CognitoIdentityProvider::Client.new(http_wire_trace: true). Super handy for dev purposes and debugging especially when some sdk methods need to do something like assuming a role first, you can verify the underlying behavior of each call. It is sometimes hard to give AWS a ton of credit, but their ruby SDK is pretty well designed IMO.