Contract.valid? does not return data about the failure

[alexevanczuk]

I'm writing a custom rspec matcher that validates the input and output arguments that are being stubbed out obey the specified contract. I'm doing this by calling Contract.valid? against the contract after pulling it using decorated_methods_for. The documentation for #valid? says that it returns metadata about the nature of the failure, but I am getting nil. I'd like the matcher to just raise the original ParamContractError when there is an error, which I'm doing by just re-calling the original method, but I'd prefer to just surface the contract failure up front explicitly.

Better yet, I'd like to just do Contract.validate!(args, contract) but this doesn't seem to be a public API. Was wondering if I could get some help with this!

[alexevanczuk]

Possibly a separate issue, but I was thinking it would be cool to send a pull request with this, along with some other work that allows contracts to play nicer with RSpec instance_double and class_double in RSpec. I'm not sure what the best way to do this would be. One way is to have a special contractual_instance_double method that returns an instance double that tricks Contracts into believing it's actually the class it's mocking (this is what we do where I work).

Such a method could also automatically inherit the same contract as the method it is mocking, so it could be used with this matcher.

Another thought is to have this custom matcher automatically search for the correct contract from the list of memoized validators if it's an instance/class double, and perhaps some sort of allow_mocks flag in Contract.valid? that allows mocks to be contractually valid.

Would love to chat more about this and what the proper way to implement this would be. I've spent some time looking through the source, so I have a couple of ideas, but if you're open to chatting more, let me know.

Here is the current state of my custom matcher:

#
# This matcher tests the boundaries for typed methods that use the Contracts gem
#
# Example:
#
#   expect(calculator)
#     .to contractually_receive(:divide)
#     .with(numerator: 1, denominator: 2)
#     .and_return(0.5)
#
#   class Calculator
#     include Contracts
#
#     Contract(KeywordArgs[numerator: Num, denominator: Num] => Num)
#     def divide(numerator:, denominator:)
#       numerator / denominator.to_f
#     end
#   end
#
# The matched object *must* be an original object, rather than an instance/class double, since these doubles
# do not inherit contracts when their methods are stubbed (this would be a possible extension)
#
# The input parameters *must* be a contract_double, since out of the box the Contracts gem does not see instance_doubles
# as being of the same type as the original objects.
#
RSpec::Matchers.define :contractually_receive do |method|
  match do |object|
    expect(object).to receive(method).with(*@params || no_args).and_return(@stubbed_value)

    type_of_methods = object.class == Class ? :class_methods : :instance_methods
    contract = Contracts::Engine::Base.fetch_from(object.class).decorated_methods_for(type_of_methods, method).last

    # It does not look like the Contracts gem has a validate! method, and while we could call the original
    # method to raise the error, there may be side effects if there are other decorators involved.  Calling the
    # original method would also not work for the return value.
    Contract.valid?((@params || [nil]), contract.args_contracts) || fail_with(:input, contract, @params)
    Contract.valid?(@stubbed_value, contract.ret_contract) || fail_with(:output, contract, @stubbed_value)
  end

  def fail_with(type, contract, actual_value)
    type_string = case type
                  when :input
                    "Param values do not match contract"
                  when :output
                    "Return value does not match contract"
                  end

    expected = case type
               when :input
                 contract.args_contracts.to_s
               when :outputq
                 contract.ret_contract.to_s
               end

    raise RspecMockParamContractError.new(
      "#{type_string}:\n \
      Expected: #{expected}\n \
      Actual: #{actual_value.to_s}\n \
      Value Guarded At: #{contract.method.method_position}")
  end

  description do
    'receive and obey contract'
  end

  chain :with do |*args|
    @params = *args
  end

  chain :and_return, :stubbed_value
end

class RspecMockParamContractError < StandardError
end

[egonSchiele]

Hello, sorry for the late response.

The documentation for #valid? says that it returns metadata about the nature of the failure, but I am getting nil

That sounds like a bug. I wonder if failure_callback would work instead?

some other work that allows contracts to play nicer with RSpec instance_double and class_double in RSpec

I'd love to talk about this more, I think a lot of people would find this useful. I'd like to do it in a way that doesn't add a dependency to the contracts lib, or introduce too much rspec-specific logic into the codebase. Based on that, the first approach you listed sounds better. I don't have a full picture of the issues here + the pros/cons of each approach, could you explain those more?

[alexevanczuk]

Hey thanks for your response @egonSchiele . I've lost some of the context, but I'll do my best to summarize the approaches.

Firstly it does sound like a bug that #valid? doesn't return the metadata. Not sure about failure_callback -- I'd have to look a bit more into that.

Regarding integrating with Rspec more nicely, off the top of my head there isn't much way around a custom matcher. Rspec's expect(object).to receive(:method) will override the method on the original object, which will also mean that any decorators, including contracts, are lost. As such if we want to verify the contract was preserved, it needs to be done in a matcher. That being said, there is an approach that is a little lighter which wouldn't require adding the rspec dependency.

Approaches

1. Define a custom rspec matcher

Pros:

• People can make immediate use of the matcher by requiring it

Cons:

• Need to require https://github.com/rspec/rspec-expectations (note that rspec breaks up its codebase into several areas, so it would just need this gem)
• Requires adding a validate! method
  It would look like this:

RSpec::Matchers.define :contractually_receive do |method|
  match do |object|
    expect(object).to receive(method).with(*@params || no_args).and_return(@stubbed_value)
    type_of_methods = object.class == Class ? :class_methods : :instance_methods
    contract = Contracts::Engine::Base.fetch_from(object.class).decorated_methods_for(type_of_methods, method).last
    Contract.validate!((@params || [nil]), contract.args_contracts)
    Contract.validate!(@stubbed_value, contract.ret_contract)
  end

  chain :with do |*args|
    @params = *args
  end

  chain :and_return, :stubbed_value
end

2. Have a helper method that validates input and output params against a class and method

Pros:

• Does not require dependency on rspec
• People can more easily integrate it into other test suite types

Cons:

• People will need to create their own custom matchers with this
• Slightly more complex
  It might look something like this. Note that we'd introduce two methods, a validate! that accepts params and a list or single contract, and validate_method! that accepts an object, the method, input, and output params. We'd have to think more about what a better interface for this would be.

def self.validate!(object, method, input_params, output_params)
  type_of_methods = object.class == Class ? :class_methods : :instance_methods
  contract = Contracts::Engine::Base.fetch_from(object.class).decorated_methods_for(type_of_methods, method).last
  Contract.validate!(input_params, contract.args_contracts)
  Contract.validate!(output_params, contract.ret_contract)
end

Consumer code bases could include this if they want:

RSpec::Matchers.define :contractually_receive do |method|
  match do |object|
    expect(object).to receive(method).with(*@params || no_args).and_return(@stubbed_value)
    Contract.validate!(object, method, @params || [nil], @stubbed_value)
  end

  chain :with do |*args|
    @params = *args
  end

  chain :and_return, :stubbed_value
end

The last part I talked about, with contract_doubles and such, would be a further extension that allows us to use matchers like this against stubs/doubles. Since stubs/doubles will not be decorated in the same way the original method will be, there will be no contracts. My proposal was basically create a contract double like this (pseudocode):

def contractual_instance_double(stubbed_class, attr_map = {})
  instance_double(stubbed_class, attr_map).tap do |dbl|
    allow(dbl).to receive(:is_a?).with(anything) { false }
    allow(dbl).to receive(:is_a?) { |tested_class| tested_class.in?(stubbed_class.ancestors) }
    allow(dbl).to receive(anything).and_wrap_original do |m, *args|
      contract = Contracts::Engine::Base.fetch_from(stubbed_Class).decorated_methods_for(: instance_methods, m).last
      # Attach contract to instance double method somehow
      # Call original method
      dbl.send(m, *args)
    end
  end
end

A con here is that it still requires an rspec dependency to create instance doubles as well as the allow methods. If we're going to have an rspec dependency anyways, I think I'd rather have it be in the custom matcher, and have the custom matcher search for the original contract dynamically against the actual class that the double is stubbed from and validate it there.