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

[WiP] Update of request specs generator due to change in rails 6.1 controller default behaviour. #2464

Conversation

panSarin
Copy link

@panSarin panSarin commented Feb 18, 2021

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

Copy link
Member

@pirj pirj left a 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.

@panSarin
Copy link
Author

panSarin commented Feb 18, 2021

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?

@panSarin panSarin changed the title Update of request specs generator due to change in rails default behaviour. [WiP] Update of request specs generator due to change in rails default behaviour. Feb 18, 2021
@pirj
Copy link
Member

pirj commented Feb 18, 2021

generate('scaffold widget name:string category:string instock:boolean foo_id:integer bar_id:integer --force')
should generate a controller and its specs. Those specs are run afterwards. It is worth checking what exactly is generated, and why generated request specs won't fail on 6.1.1.

@panSarin
Copy link
Author

Thanks for the hints! I am on it.

@panSarin panSarin changed the title [WiP] Update of request specs generator due to change in rails default behaviour. [WiP] Update of request specs generator due to change in rails 6.1 controller default behaviour. Feb 18, 2021
@panSarin
Copy link
Author

@pirj so I tried to test it by example app, and I encounter problems with generating example app for rails 6.1
I found there is opened #2398 PR for that. Is it planned to be merged soon or I should create that PR for that branch?

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.

@panSarin panSarin force-pushed the issue-2463-scaffold-for-request-specs-update branch 2 times, most recently from 594aa89 to 1c696f4 Compare February 18, 2021 14:19
@pirj
Copy link
Member

pirj commented Feb 18, 2021

Ouch. Sorry, I forgot to point out that we have a branch for 6.1 fixes, rails-6-1-dev.
We've just one fix left to merge, #2461, and hopefully, it is ready.
Your fix can be part of that branch. Please let me know if you can change the base branch:
2021-02-18_17-53-28
or I can do it for you.

and fill invalid attributes

I would suggest gsub'ing required files as we do here

gsub_file 'spec/spec_helper.rb',

@pirj
Copy link
Member

pirj commented Feb 18, 2021

That looks better! 👍
Just need a test to prove it fixes it for 6.1.1 and doesn't break for previous ones.

@panSarin
Copy link
Author

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 ;]

@pirj
Copy link
Member

pirj commented Feb 18, 2021

rspec-rails/example_app_generator/generate_stuff.rb should generate a controller and its specs. Those specs are run afterwards. I would suggest gsub'ing those generated files

to include necessary validation for 422 to be triggered.
That's it!

pirj and others added 4 commits February 19, 2021 15:34
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
@panSarin panSarin changed the base branch from main to rails-6-1-dev February 19, 2021 14:35
@panSarin panSarin force-pushed the issue-2463-scaffold-for-request-specs-update branch from 1be7158 to d98ec7a Compare February 19, 2021 14:36
@panSarin
Copy link
Author

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

@panSarin panSarin closed this Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants