Migrate controller specs to requests specs

[sethherr]

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

[sethherr]

The one issue with consolidating into request specs (rather than moving request specs into controller specs) is that when moving the controller specs, we will need to specify that they are controller specs - which will be a larger number of additions

[sethherr]

From rspec-rails response to "Are controller specs discouraged in favor of request specs?"

The best advice we can give for people working on rails apps is to write request specs instead of controller specs. I'd consider writing a new controller spec in 2017 to be a smell, even on an older rails app. Keeping controller specs around for legacy reasons is absolutely fine though.

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 assign as an important part of their test functionality would be a loss.

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 ?

[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 :)

[sethherr]

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 :/

[jmromer]

so renaming our request specs into controller specs makes us look like we aren't following current best practices.

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 type: annotation and how the test is written does (it would not be the first time I've seen "controller" specs written as request specs and housed in spec/controllers), but I agree I don't think this is worth dwelling on much.

[sethherr]

This is easily possible when we upgrade Rails, since you can check assigns in rails 5 request specs. So we're waiting on that before worrying about this.