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

Proposal: improve controller's shared specs #486

Open
sharipov-ru opened this issue Oct 26, 2016 · 1 comment
Open

Proposal: improve controller's shared specs #486

sharipov-ru opened this issue Oct 26, 2016 · 1 comment
Labels

Comments

@sharipov-ru
Copy link
Contributor

sharipov-ru commented Oct 26, 2016

Currently we have shared specs which define lists of params for different groups: unavailable_params_list, error_params_list and so on. Example:

it_behaves_like "supplier quote method" do
  let (:supplier_client) { stubbed_client }
  let(:success_params) {
    { property_id: "success", check_in: Date.today + 10, check_out: Date.today + 20, guests: 2 }
  }
  let(:unavailable_params_list) {[
    success_params.merge(property_id: "unavailable")
  ]}
  let(:error_params_list) {[
    success_params.merge(property_id: "malformed_response"),
    success_params.merge(property_id: "timeout")
  ]}
end

With that format shared spec is implemented in the way when it checks just the successfulness of the Result:

  context "when errors occur" do
    it "returns the error result" do
      error_params_list.each do |params|
        quotation_result = supplier_client.quote(params)
        expect(quotation_result).not_to be_success
        expect(quotation_result.error).to_not be_nil
      end
    end
  end

Which makes it hard to check for the existence of specific error.code and error.data. Currently we have to remove specific case from params_list and implement separate spec. Probably we can change error_params_list format to support for expected error.code and error.data:

 let(:error_params_list) {
   [
    { params: { ... }, code: :stay_too_short, data: "The minimum number of nights to book this apartment is 10" }
    { params: { ... }, code: :check_in_too_near, data: "Selected check-in date is too near" },
    { params: { ... }, code: :timeout, data: nil }
  ]
}
@kkolotyuk
Copy link
Contributor

I agree that we should add code and data to the error_params_list but not sure I understand this:

Currently we have to remove specific case from params_list and implement separate spec.

Why we can't add new spec and don't remove it from the list?

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

No branches or pull requests

2 participants