spock icon indicating copy to clipboard operation
spock copied to clipboard

Mocking: Variables of Closure type aren't evaluated as code args

Open jonbarksdale opened this issue 9 years ago • 4 comments

    def beatIt = {
      def result = (it >= now)
      Thread.sleep(1)
      now = System.currentTimeMillis()
      return result
    }

    when:
    task.execute(context)

    then:
    1 * loggingService.startJob(job, _, beatIt) >> jobExec    // doesn't work
    1 * loggingService.startJob(job, _, { beatIt(it) }) >> jobExec // works

Example is from one of my tests, where I discovered it.

From what I can tell, it is related to how you decide what is a code argument in the InteractionRewriter.addArg method. Maybe try also inspecting any VariableExpressions for closure type?

jonbarksdale avatar May 31 '16 23:05 jonbarksdale

We saw this too, a consequence of our move to Java 8!

Try strongly typing your "def result" declaration with the parameter type of the third parameter to loggingService#startJob! (e.g. Function result = { it >= now })

dwoldrich avatar Jun 08 '16 19:06 dwoldrich

@dwoldrich I'm not sure how strongly typing the def result to Long would help, given that I am storing a boolean there. I have also tried strongly typing the closure parameter:

    def beatIt = { Long it ->
      def result = (it >= now)
      Thread.sleep(1)
      now = System.currentTimeMillis()
      return result
    }

as well as casting the closure itself to Closure and Closure<Long>, and nothing works. Again, from what I read in the InteractionRewriter class, it seems to be with how Spock handles the mock:

private void addArg(Expression arg) {
    if (arg instanceof NotExpression) {
      NotExpression not = (NotExpression)arg;
      addArg(not.getExpression());
      call(InteractionBuilder.NEGATE_LAST_ARG);
      return;
    }

    if (arg instanceof CastExpression) {
      CastExpression cast = (CastExpression)arg;
      addArg(cast.getExpression());
      call(InteractionBuilder.TYPE_LAST_ARG, new ClassExpression(cast.getType()));
      return;
    }

/* This if is the issue.  If it is not a closure literal, it doesn't trigger. 
In my above example, the arg is an instance of VariableExpression, not Closure expression.
This makes it a little dicey in the case of actual closure typed args, but it seems like there should be a better way to handle this.
*/
    if (arg instanceof ClosureExpression) {
      call(InteractionBuilder.ADD_CODE_ARG, arg);
      return;
    }

    call(InteractionBuilder.ADD_EQUAL_ARG, arg);
  }

When I have time, I'll look into writing a patch, but it would be good to decide what the best behavior is in the case of a method that actually has a Closure typed argument.

jonbarksdale avatar Jun 08 '16 19:06 jonbarksdale

I misread your original example, I thought def result was supposed to be a closure, but beatIt is the closure.

What happens when you strongly type beatIt as a Function?

Function beatIt = {
  def result = (it >= now)
  Thread.sleep(1)
  now = System.currentTimeMillis()
  return result
}

It seemed to work for me when I used Function as the strong type name in my code for my Groovy closure, but Closure is probably the correct type there. I suppose Groovy is able to coerce or proxy Closure to java.util.function.Function, which is what is happening in my code and everything still works.

I just attempted to change the strong type of my Groovy closure from Function to Closure and got a Condition not satisfied error in my test. I suspect the strong type you use for the variable that holds the reference to the closure in your test needs to match the parameter type on the signature of the method that you are mocking/spying. In my case, the function I'm mocking has a java.util.function.Function as its parameter and my strong type on my closure needed to match it.

dwoldrich avatar Aug 10 '16 19:08 dwoldrich

Maybe the docs in http://spockframework.org/spock/docs/1.1/all_in_one.html#_argument_constraints should be improved to state that it requires a ClosureLiteral and not just an instance of the closure to be treated as a code argument constraint.

leonard84 avatar Jul 08 '18 14:07 leonard84