Skip to content
New issue

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

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

Already on GitHub? # to your account

Add test for declared params with multiple types #2117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Oct 9, 2020

Adds tests from #2112

@grape-bot
Copy link

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
📖 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 🚫 danger

@dblock
Copy link
Member

dblock commented Oct 9, 2020

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?

@stanhu
Copy link
Contributor Author

stanhu commented Oct 26, 2020

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:

# Currently supported group types are Array, Hash, JSON, and Array[JSON]

Do we need a test for this?

@stanhu
Copy link
Contributor Author

stanhu commented Oct 26, 2020

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

@dblock
Copy link
Member

dblock commented Nov 5, 2020

@stanhu want to finish this?

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

Successfully merging this pull request may close these issues.

3 participants