Keyword arguments’ defaults fixes

[chastell]

https://github.com/psyho/bogus/pull/4 made it possible to work with methods that take keyword arguments, but with that implementation the faked methods can’t be called with some of the arguments missing.

This takes the first step in making this possible by defaulting all kwargs to nil (much like all optional arguments are defaulted to {}). This is not a final solution, as have_received needs to explicitly expect a nil-ed default, but it’s better than what’s currently on master:

require 'bundler/setup'
require 'bogus/rspec'

class Registrar
  def register client: raise, repos: {}
    puts "registering #{client} into #{repos}"
  end
end

class Handler
  def handle client, registrar: Registrar.new
    registrar.register client: client
  end
end

describe Handler do
  describe '#handle' do
    it 'handles RGST inputs' do
      registrar = fake :registrar
      Handler.new.handle 'client', registrar: registrar
      registrar.should have_received.register client: 'client', repos: nil
    end
  end
end

Note how the repos keyword argument is nil in the have_received call, while it’s {} in the actual kwarg default; still, the above spec at least passes, rather than blowing up.

[coveralls]

Coverage remained the same when pulling b47b14027ec1e97b987f605e1dc8c04e2a46cac4 on chastell:keyword_arguments_defaults into 98f69d579908bd2bc5c2d2a65e73d152b29d6c48 on psyho:master.

[wrozka]

Thanks for the commits, but we need some specs for the 2.0 support. Could you add a conditional test that runs only on 2.0 in mocking dsl spec?

I have an idea for better support for named arguments, since they are optional, we shouldn't require users to pass default value in expectations. I think we could change the method_stringifier to when :key then "#{name}: Bogus::Anything" and update the interaction's equals method. I will experiment with that later today.

[chastell]

Thanks for looking into this! Adjusting Interaction#same_args? might indeed work. :)

I’ll come up with specs, but it’s going to be a busy week for me, so probably not earlier than over the weekend.

[wrozka]

We've found out there was a bug in optional arguments handling, fixed in: https://github.com/psyho/bogus/commit/ea0a8dbcdd2cf8f856d79406b5eebdfca3cd9673 We spiked the keywords support, it is not so complicated, but still requires some love. I hope to find time for that this week.

[chastell]

Switched kwarg default value handling to DefaultValue, will work on the specs over the weekend. Of course feel free to implement your own solution and close this PR, I just needed this for my current project anyway . :)

BTW: What’s your stance on rebasing PRs vs merging master into them? (This one needed changes from master and couldn’t be rebased cleanly anyway, but it’d be great to know for the sake of other PRs.)

[wrozka]

I'm rebasing all the things everyday at work and I like when history is simple, so I avoid merges as much as I can. Feel free to rebase it and do push --force as long its not in the master ;)

If you have time for finishing this PR, please do. I will configure coveralls and help a friend that is migrating from previous version of bogus this weekend.

[chastell]

I probably overdid this, but let me know what you think – spec/bogus/mocking_dsl_kwargs.rb contains adapted specs from spec/bogus/mocking_dsl_spec.rb (minus the parts that are verified by it anyway) and is conditionally loaded if a Ruby version is supposed to be able to parse it.

[wrozka]

I've commented two last things and then we can merge. Thank you. :)

[wrozka]

To fully support Ruby 2.0, we'll need to handle double splat, so the stubbing with wrong keyword names will fail as expected. I've added a ticket #12 for that.

[chastell]

Not too hapy about the Elvis operators, but they’re less gross than the gsub calls.

default could be set to equal name to drop the :key-related logic, but then :opt-related logic gets more confusing and not worth it (IMHO):

    def argument_to_string(name, type, default)
      case type
      when :block then "&#{name}"
      when :key   then "#{name}: #{default}"
      when :opt   then name == default ? name : "#{name} = #{default}"
      when :req   then name
      when :rest  then "*#{name}"
      else raise "unknown argument type: #{type}"
      end
    end

[wrozka]

I don't mind the current use of Elvis, it is readable for me and we've got rid of the ugly gsub, thanks. :)

[chastell]

Trimmed down the keyword argument specs and rolled into the main MockingDSL spec file. :)

[wrozka]

Thanks. After we add double splat support #12, we can ship Bogus to 2.0 users.