-
Notifications
You must be signed in to change notification settings - Fork 70
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
Migrate controller specs to requests specs #845
Comments
|
From rspec-rails response to "Are controller specs discouraged in favor of request specs?"
I'm updating and renaming this issue to reflect this. I don't think we should move the controller specs into the request specs folder, because they are different and removing all of the tests that use I'm going to call controller specs deprecated, no new controller specs should be added and whenever touching existing controller specs, strongly consider moving them into request specs. Let me know if that sounds good @jmromer ? |
I don't think I follow, but it's less time-consuming to let the code speak than disambiguate natural language in this case — have a look at #848, and feel free to close if it doesn't seem like a good move right now. Definitely won't mind :) |
The issue I see with just moving the specs around is that controller specs should no longer be used (according to ruby and rspec, visible in "are controller specs discouraged") - so renaming our request specs into controller specs makes us look like we aren't following current best practices. I think it would make it easier to find specs at the cost of it being explicit which thing is which and how far we are along on this migration. Immediately refactoring the controller specs into request specs seems pretty low priority to me. I would prefer the incremental approach, because it means that, hopefully, we'll be focused on the areas that we're refactoring the tests for when we refactor them (because we'll be touching the area). The disadvantage I see to not immediately refactoring means that this "No new controller specs should be added and whenever touching existing controller specs, strongly consider moving them into request specs" is institutional knowledge rather than code :/ |
My position is that the location or the name of the file doesn't / shouldn't determine whether it's a request or controller spec, the |
This is easily possible when we upgrade Rails, since you can check |
I kept assigns base on bikeindex/bike_index#845 (comment) to not lose important part of test functionalities
I kept assigns base on bikeindex/bike_index#845 (comment) to not lose important part of test functionalities
As per #837 (comment), we're moving toward request specs.
No new controller specs should be added and whenever touching existing controller specs, strongly consider moving them into request specs
The text was updated successfully, but these errors were encountered: