-
-
Notifications
You must be signed in to change notification settings - Fork 40
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: Use conventions to define the schema path #22
Comments
@maurogeorge Thanks for opening this ticket. This approach is very interesting. I'm curious if this convention could be inferred outside a Rails app (Sinatra, Grape, or other Rack apps). Would you be willing to flesh out the implementation you're considering? |
@seanpdoyle Yeah I really don't know if this convention could be inferred outside of Rails. But in any case we can start doing this behavior only on Rails apps and in other Rack apps the user need to pass the params explicitly. What you think? |
@maurogeorge That's true. I think it depends on how robust the implementation is, and the sorts of patterns consumers use for declaring their schema. Would you be willing to work a PR with an initial implementation or some failing test cases? |
This approach sounds interesting, but to me it sounds a little magical. I tend to avoid magic as much as I can in my tests, and I really like the current approach that we're taking with this matcher because it's easy to see which schema file is going to be referenced. If the test fails in some manner and the test doesn't have some kind of hint as which file contains the error, then it's going to be difficult to debug the test, especially for people who are not familiar with this matcher and using it for the first time. (I like to make tests as easy to debug as possible too.) As far as merging this in with shoulda-matchers, we intentionally decided to keep this separate as we didn't really know if it would fit in and we figured we'd have to play around with it first internally. We may decide to merge it in the future, but for now I think it's best if we keep it separate. |
@seanpdoyle Yeah I can work on a PR, maybe in the end of the next week, but after the @mcmire comments do you belive that's make sense this PR or prefer keep the things the way it is? @mcmire thanks for your feedback. I understand your point, make the things explicit has values too, it is about trade-offs, if the file is a param we can have a folder with all schemas or a confusing organization, following a convention we try to avoid this. |
Hi,
Recently I make a proposal to create a test helper on the AMS, that follows the same idea, use a schema file to validate a API response. But in my case I am using conventions to load the schema file. Example
I remove the param of the schema path, and assumes that the schema file lives in
spec/support/api/schemas/posts/index.json
since it is theposts#index
tests and make the param optional. Makes sense to you guys?A little doubt, Makes sense this be a matcher on the shoulda-matchers? This way we will need only a dependency and we can use this with the Minitest too. I think we can handle this as a Action Controller matcher. @mcmire makes sense?
Thanks guys
The text was updated successfully, but these errors were encountered: