Consider providing a way to define jMock-style custom actions and/or rspec-mocks block implementation

[floehopper]

See http://www.jmock.org/custom-actions.html and https://relishapp.com/rspec/rspec-mocks/docs/configuring-responses/block-implementation.

[nitishr]

I'm not sure I understand custom actions properly. With my limited understanding, I suspect #379 may provide the described capability.

If not, I'd love to help implement this feature if you could explain to me the desired behavior. A ruby/mocha hypothetical example would be great.

[floehopper]

I've changed the title of this issue, because I want to use it to roll up a number of closely related issues and have the discussion in one place.

[floehopper]

I'm not sure I understand custom actions properly. With my limited understanding, I suspect #379 may provide the described capability.

If not, I'd love to help implement this feature if you could explain to me the desired behavior. A ruby/mocha hypothetical example would be great.

@nitishr Thanks for asking about this.

You're right that #213, #230, #231, #317 and your latest #379 are all closely related to this issue. I'm going to close all those and continue the discussion here.

[floehopper]

My hesitation about implementing this seemingly innocuous extra functionality is that it seems to violate the idea of a stub always returning "canned" data and thus risk the test becoming very complicated and hard to follow.

Passing a block or proc to be invoked by the stub means allowing any amount of arbitrary code to be executed by the stub including the possibility of mutating state outside the stub or even global state. This feels inherently dangerous and I can imagine people getting in a real mess with it.

I think that the execution flow of a test should go from the test (T) to the object under test (O) and then into one or more stubbed methods (S) and then back out to the object under test and ultimately the back to the test again:

T -> O
     O -> S
     O <- S
T <- O

However, if you allow an arbitrary block to be invoked by the stub, you're likely to end up with something more like:

T -> O
     O -> S
T <------ S
T ------> S
     O <- S
T <- O

And I think this would quickly become hard to follow.

My thinking on this is somewhat influenced by the mess many people get into by using Mocha::ClassMethods#any_instance. I think this is rarely needed and is often the sign of a problem with the design, although in the case of testing legacy code or code that you don't have control over, I can see it has benefits. I sometimes think it might make sense to have a separate "legacy code" mode for Mocha which enables some features which are disabled by default. This is somewhat related to the stubbing_method_on_non_mock_object configuration option.

The reason I thought basing functionality like this on jMock's custom actions was it might be a way to constrain the functionality in a way that made it clear how it was meant to be used and make it less likely people would get in a mess.

Looking through the block implementation examples in the rspec-mocks documentation it feels as if most of those scenarios can be handled in another way:

• Use a block to specify a return value with a terser syntax
  • This doesn't seem like a strong motivator to me. Mocha's syntax is already terser than rspec-mock's in this area anyway.
• Use a block to verify arguments
  • This is better achieved by writing a custom matcher by passing a block to Mocha::Expectation#with.
• Use a block to perform a calculation
  • This can be achieved by stubbing the same method multiple times with different arguments to Mocha::Expectation#with. I can see how this functionality could reduce duplication if you're stubbing the calculation method many times, but I think the same reduction of duplication could be achieved by extracting a method to do setup the stubbing.
• Yield to the caller's block
  • This can be achieved using Mocha::Expectation#yields and/or Mocha::Expectation#multiple_yields.
• Delegate to partial double's original implementation within the block
  • I suspect that in most/all cases, you'd be able to re-design the implementation to make it easier to test. If the code under test is legacy code and you can't change it, then you're probably better off using some kind of custom fake object rather than stubbing with Mocha.
• Simulating a transient network failure
  • This can be achieved by multiple calls to Mocha::Expectation#raises and/or Mocha::Expectation#returns.

Thus I don't currently see a strong argument for introducing the functionality. If anything, I think the best solution might be to improve the documentation to provide suggested solutions for the examples in the rspec-mocks docs and examples from some of the PRs & issues related to this one.

I welcome any thoughts people have on the subject.

[Hubro]

@floehopper My use case for this is usually to replace a function that does IO with a function that returns mock data based on the input arguments. The mock function is often as simple as serializing the input arguments and using the result to return a value from a hash. Is there a simple way to achieve this without the ability to provide the mock implementation?

[floehopper]

@Hubro Interesting. Any chance you could post some simple example code here, so I know exactly what you mean?

[aditya87]

@floehopper The simplest usecase for this I can think of is dynamically controlling the returned value as a function of the argument passed in when that transformation cannot be modeled as a limited set of if-then arguments. Like what if I want to stub a method to just return what's passed in? E.g.

Foo.stubs(:bar) do |arg_1|
  arg_1
end

etc.

[nhorton]

I have another example where I need this. I need to stub a method that could take some time, and I want to call the rails travels time helper to simulate the method taking a while

[floehopper]

I have another example where I need this. I need to stub a method that could take some time, and I want to call the rails travels time helper to simulate the method taking a while

@nhorton Hmm. That's an interesting use case. Do you have a more concrete example of where it's important that the stubbed method looks like it's taking some time to execute - ideally some actual test and implementation code. If I can't think of a way to redesign the implementation to make it easier to test, then I'll definitely consider adding a feature to Mocha to do this, although I'd probably lean towards adding something very specific to this use case (e.g. foo.stubs(:bar).after(5.seconds).returns(:bar) rather than allowing arbitrary block evaluation.

[floehopper]

@floehopper The simplest usecase for this I can think of is dynamically controlling the returned value as a function of the argument passed in when that transformation cannot be modeled as a limited set of if-then arguments. Like what if I want to stub a method to just return what's passed in? E.g.

Foo.stubs(:bar) do |arg_1|
  arg_1
end

etc.

@aditya87 I'm so sorry that it's taken me sooo long to reply to your comment. I'm really looking for a more concrete example than that - ideally both test and implementation code. That way I can see if the problem can be addressed by redesigning the implementation so that it can be tested with the existing Mocha features. If I can't then that's more motivation to add a feature to Mocha and would give me a clearer idea of the problem I'm trying to solve with that feature. I hope that makes sense.

[nhorton]

Here is an example. I think this might be worst-case scenario too. The functionality in question here is a soft-timeout on jobs where they end and re-enqueue themselves (with checkpoints) when an inner loop has taken longer than the allowed time. The 37Signals blog has an article on this approach in their blog. This code is stupid-dumb because I need to keep everything in the same context to get the time travel functionality in Rails test to work properly.

The test code is awkward as I wanted to use a Mocha mock object but had to pivot to injecting a module into Integers when Mocha did not work.

  module NumericWarmeable
    def warm_record_cache!(async:)
      raise "Should not be async" if async
      stubbed_travel self.seconds # Travel is the rails time travel testing helper
    end

    # This has to exist to be stubbed, and we need to stub it with a proc for scope purposes
    def stubbed_travel(seconds) = 1
  end

  test "the soft time limits work - slow tasks" do
    instant = warmeables(:example_1)
    Warmeable.any_instance.expects(:warm_record_cache!).times(2)

    # This is dumb-complex to test as we have to stub awkwardly to have the time travel work
    too_long = 10.minutes.seconds
    too_long.extend(NumericWarmeable)
    mock_warm = proc { |seconds| travel seconds }
    too_long.stub(:stubbed_travel, mock_warm) do
      assert_enqueued_jobs(1, only: CacheWarmingJob) do
        CacheWarmingJob.perform_now(@account, [instant, too_long, instant])
      end
      perform_enqueued_jobs(only: CacheWarmingJob)
      assert_enqueued_jobs(0, only: CacheWarmingJob)
    end
  end

[floehopper]

@nhorton Thanks. I hope you don't mind but I've fixed the indentation in your code example. Also I think I'm missing a few things here to be able to understand & run this code, e.g. the definitions of Warmeable, CacheWarmingJob, #warmeables.

[nhorton]

Scan https://dev.37signals.com/making-export-jobs-more-reliable/ for a minute. That is the core of what I am doing.

To directly answer, Warmeable in my example is a basically a delegated type that needs a bunch of computations done on it. The job in question does the computations on a set of objects. The issue is that some computations are fast and some are slow (they call out to remote systems ) and I need to simulate different response times. But because I can't properly do a mock, and because Active job serialization does not support arbitrary types, I ended up extending int in a stupid-complex way trying to get something that could stub with a proc that has the rails time travel methods in scope and would serialize.

On Mon, Sep 4, 2023, 2:47 PM James Mead @.***> wrote:

@nhorton https://github.com/nhorton Thanks. I hope you don't mind but I've fixed the indentation in your code example. Also I think I'm missing a few things here to be able to understand & run this code, e.g. the definitions of Warmeable, CacheWarmingJob, #warmeables.

— Reply to this email directly, view it on GitHub https://github.com/freerange/mocha/issues/224#issuecomment-1705689609, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABR24TJ77I7OHXIJ2ELQM3XYY46XANCNFSM4A6SFUFA . You are receiving this because you were mentioned.Message ID: @.***>

[floehopper]

Hmm. It would really help me if you could at least sketch out the classes/modules under test, even if the method bodies are just comments explaining roughly what they do. What might seem obvious to you, isn't to me!

Also I'm confused by this method call:

too_long.stub(:stubbed_travel, mock_warm) do
  # ...
end

That stub doesn't seem to be a legitimate Mocha method and even if it's a typo of stubs, that doesn't accept a block. Or am I missing something...?

[nhorton]

Explaining my code will never be ideal as there is too much domain stuff. Let me do a simpler synthetic version with the code I would like to write.

• Imagine you have a remote service that takes stillframes of videos and does some cool AI manipulation on it
• Call that API ImageService.capture_frame(video_name)
• Now imagine that service takes between 1 and 60 seconds to complete that call and it is synchronous, and you pay by the second
• Now imagine that in your product has a free tier where you let people use up to 5 minutes of that service per day, then you cut them off until the next day.

An example test that I can write now is on the daily reset. Roughly

test "Test that the reset happens right at midnight regardless of when the last call was" do
  travel_to Time.current.midnight - 1.hour
  test_account.over_daily_limit = true
  travel_to Time.current.midnight + 1.second
  assert_false test_account.over_daily_limit
end

But what I can't do is something that stubs that service for testing my tracking.

test "When I cross 5 minutes with ImageService, the account is marked over its limit" do
  ImageService.stubs(:capture_frame).latency(30.seconds).returns("https://fake.url/image.jpb")
  test_account.usage_today = 4.minutes + 45.seconds
  test_account.encode_another_video # This is the method that internally calls ImageService.capture_frame
  assert_true test_account.over_daily_limit
end

Thanks for having follow-up here.