Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rspec respond_to not functional with required keyword in call to new #1071

Closed
liamelliott opened this issue Sep 6, 2018 · 5 comments
Closed

Comments

@liamelliott
Copy link

liamelliott commented Sep 6, 2018

I have some code using rspec version 3.8 that doesn't produce the result expected at this link. If I create another class method with the same keywords and behaviour, it works without any issues.

class Visit

  Facility = Struct.new(:code, :name) do
    def to_facility
      self
    end
  end

  attr_reader :facility, :start_date, :number_of_nights

  def initialize(facility:, start_date: nil, number_of_nights: 1)
    @facility = facility
    @start_date = start_date
    @number_of_nights = number_of_nights
  end

  def self.other(facility:, start_date: nil, number_of_nights: 1)
    @facility = facility
    @start_date = start_date
    @number_of_nights = number_of_nights
  end

end
describe Visit do

  describe ".initialize" do
    it "correctly responds to :new" do
      expect(described_class).to respond_to(:other).with_keywords(:facility) # Works!
      expect(described_class).to respond_to(:new).with_keywords(:facility) # Doesn't work!
    end
  end
end

This test fails with "expected Visit to respond to :new with keyword :facility," but I don't see how my usage is any different from the relishapp example docs (1 required and 2 other keyword arguments). Only in their case they're using plant(seed:, fertilizer: nil, water: 'daily).

@liamelliott liamelliott changed the title Rspec respond_to not functional with required keyword in initialize Rspec respond_to not functional with required keyword in call to new Sep 6, 2018
@myronmarston
Copy link
Member

The underlying issue is that ruby defines Visit.new to accept all arguments and pass them through to #initialize. As a result, the metadata available about Visit.new does not indicate anything about the #initialize arguments:

2.4.4 :018 > Visit.method(:new).parameters
 => [[:rest]]
2.4.4 :019 > Visit.instance_method(:initialize).parameters
 => [[:keyreq, :facility], [:key, :start_date], [:key, :number_of_nights]]

We solved this for rspec-mocks verifying doubles by looking at the signature of #initialize when dealing with new:

rspec/rspec-mocks#886

We could theoretically do the same for respond_to but someone will have to invest the effort to do that.

@liamelliott
Copy link
Author

@myronmarston Interesting, thanks. Instead of a direct workaround of the ruby functionality on sending :new, would you support adding something like expect(described_class).to respond_to(:new).and_defer_to(:initialize).with_keywords(:facility)?

@myronmarston
Copy link
Member

IMO, that's a worse API, and only solves the problem for developers who are aware of the issue. If we're going to solve this, I think we should automatically use the parameters for initialize for respond_to(:new), similar to what we did for verifying doubles.

@JonRowe
Copy link
Member

JonRowe commented Sep 13, 2018

I took the time to implement this as #1072

@myronmarston
Copy link
Member

Closing since @JonRowe implemented this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants