grape icon indicating copy to clipboard operation
grape copied to clipboard

Add test for declared params with multiple types

Open stanhu opened this issue 3 years ago • 5 comments

Adds tests from https://github.com/ruby-grape/grape/pull/2112

stanhu avatar Oct 09 '20 21:10 stanhu

1 Warning
:warning: Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
:book: We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Here's an example of a CHANGELOG.md entry:

* [#2117](https://github.com/ruby-grape/grape/pull/2117): Add test for declared params with multiple types - [@stanhu](https://github.com/stanhu).

Generated by :no_entry_sign: danger

grape-bot avatar Oct 09 '20 22:10 grape-bot

my question was also whether it matters if we put Hash in requires :bar, type: Array do, so whether we need to have this test try both Hash and Array?

dblock avatar Oct 09 '20 22:10 dblock

my question was also whether it matters if we put Hash in requires :bar, type: Array do, so whether we need to have this test try both Hash and Array?

@dblock If I use requires :bar, type: Set, I get:

     Failure/Error: raise Grape::Exceptions::UnsupportedGroupTypeError.new unless Grape::Validations::Types.group?(type)
     
     Grape::Exceptions::UnsupportedGroupTypeError:
       group type must be Array, Hash, JSON or Array[JSON]

It seems that Hash and Array are not valid group types: https://github.com/ruby-grape/grape/blob/f3073e6123a55e4ec9faa268b741d28502d2dcbc/lib/grape/validations/types.rb#L114

Do we need a test for this?

stanhu avatar Oct 26 '20 07:10 stanhu

Oh, you were asking about Hash. Let me look at that.

stanhu avatar Oct 26 '20 07:10 stanhu

@stanhu want to finish this?

dblock avatar Nov 05 '20 22:11 dblock