AmbiguousDemonstrationError when incorrectly using of_next vs replace and class has matching instance method

[calebhearth]

Given a class which has matching instance and class methods, if a test incorrectly calls Mocktail.of_next instead of Mocktail.replace then tries to demonstrate a call to the method, this error is raised:

`stubs` & `verify` expect exactly one invocation of a mocked method, but 2 were detected.
As a result, Mocktail doesn't know which invocation to stub or verify. 
(Mocktail::AmbiguousDemonstrationError)

I'd expect there to be 0 invocations detected, but even better I'd love to be told that I'm not demonstrating using anything that's been mocked yet.

Here's a basic reproduction:

ambiguous.rb:

require "mocktail"

class Fighter
  def self.weapon
    new.weapon
  end

  def weapon
    "sword"
  end
end

Mocktail.of_next(Fighter)

Mocktail.stubs { Fighter.weapon }.with { nil }

$ ruby ambiguous.rb

[searls]

Ah, this is slightly thornier and worse than you might think. It's actually got nothing to do with singleton/instance method shadowing nor about the method being on the same.

This script will fail in exactly the same way:

class Steward
  def retrieve_fighter_weapon
    Fighter.new.weapon
  end
end

class Fighter
  def weapon
    "sword"
  end
end

Mocktail.of_next(Fighter)
steward = Steward.new

Mocktail.stubs { steward.retrieve_fighter_weapon }.with { nil }

Here's what's happening:

1. Mocktail.of_next(Fighter) will do a one-time stubbing of Fighter.new and—the next time it is called—will return a pre-instantiated fake instance of a Fighter (a reference to the same fake fighter instance is returned by of_next but wasn't assigned in your case because you actually meant to call replace and call a singleton method)

2. The real method you did call (Fighter.weapon) happens to be inside a stubs block, which demands that exactly one fake method is invoked, because the API needs to know "what is the call being stubbed" so it can know where to apply the with block to

3. So when you call Fighter.weapon, it's actually calling two faked methods: (1) Fighter.new (returning the fake Fighter instance) and then (2) Fighter#weapon on that fake instance. That's what triggers the error.

Here's the state of the internal storage of fake calls during the run; it sees two calls just happened instead of one, which is what's triggering the raise

cabinet.calls
=> [#<struct Mocktail::Call
  singleton=true,
  double=StubTest::Fighter,
  original_type=StubTest::Fighter,
  dry_type=StubTest::Fighter,
  method=:new,
  original_method=nil,
  args=[],
  kwargs={},
  block=nil>,
 #<struct Mocktail::Call
  singleton=false,
  double=#<Mocktail of StubTest::Fighter:0x0000000104730208>,
  original_type=StubTest::Fighter,
  dry_type=#<Class for mocktail of StubTest::Fighter:0x00000001047308e8>,
  method=:weapon,
  original_method=
   #<UnboundMethod: StubTest::Fighter#weapon() /Users/justin/code/testdouble/mocktail/test/safe/stub_test.rb:453>,
  args=[],
  kwargs={},
  block=nil>]

I don't know what we can do to avoid this, short of using a Ruby parser to read the literal code string inside each demonstration block and then applying some additional intelligence/heuristics, but that'd probably double the complexity of the library and would always be defeated by method extraction and metaprogramming.

[calebhearth]

I wonder if there’s a way to check that the mocked method is / is not called for the first time in the stubs block, and warn if it is.

[searls]

@calebhearth the tricky thing there is that for cases where the call is intended to be verified, it would be appropriate for it to not be called the first time in a stubs block, but rather by the subject.

I think one thing we definitely could do is improve the error message to be way more clear about which failure mode they're seeing, splitting it up into "hey in this stubs call, we didn't detect any mock interactions and that's a bug" vs "hey in this verify you actually called two mocked methods, X and Y and that's a bug"