-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[WiP] Update of request specs generator due to change in rails 6.1 controller default behaviour. #2464
[WiP] Update of request specs generator due to change in rails 6.1 controller default behaviour. #2464
Conversation
ff22252
to
4ccc002
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We aim to support Rails 5.0, 5.1, 5.2 and 6.0 as well (actually, even 4.2 should probably work). Will the generator work in those versions correctly?
Do you mind adding a test that would run this generated test?
I'm mostly interested in reproducing the failure you observe in 6.1.1, that would fail without your fix.
Oh that is true. I should think about backwards compatibilty too. Let me figure out how to check version of rails that we generating tests for and update that. Also any hint if there are such tests that i can base on to add test to that change? |
|
Thanks for the hints! I am on it. |
@pirj so I tried to test it by example app, and I encounter problems with generating example app for rails 6.1 Also to make that test fail there is one more step to reproduce: we have to add some model validation and fill invalid attributes, because by default they are skipped and there is no validation, so we will never go into "bad path". But still I think that is a case to fix - pleace correct me if I am wrong. |
594aa89
to
1c696f4
Compare
Ouch. Sorry, I forgot to point out that we have a branch for 6.1 fixes,
I would suggest
|
That looks better! 👍 |
Test to prove it fixes it for 6.1.1 - what you mean by "test" there, some unit test, or more like recorded video where i generate app for 5.x and 6.0 versions and it generates old version ( which works ) and another for 6.1.1 where it works with new version ? I am not familiar with testing tools that generate tests ;] |
to include necessary validation for 422 to be triggered. |
RSpec 4 has removed OperatorMatcher, as it was only supported by the should syntax, and not expect syntax.
attributes - to respond with 422 status, we have to update our scaffold for generating specs for Create and Update with invalid attributes. I updated both generators to check if controller responds with status 422 instead of 200. Applied that change only for rails 6.1.1 and higher
prepare valid and invalid attributes for widgets
1be7158
to
d98ec7a
Compare
Due to mess i had in commits and branches, i opened new clear PR so I am closing that and moving further conversation to #2466 |
Since rails 6.1.1 changed default behaviour of CRUD response for invalid
attributes - to respond with 422 status, we have to update our scaffold
for generating specs for Create and Update with invalid attributes.
I updated both generators to check if controller responds with status
422 instead of 200.
Also, I didn't find any tests about those generators, so I couldn't update them - if there are some, please let me know so I can update them.
Linked issue: #2463