Migrate Controller Specs to Request Specs

[armahillo]

We have dozens (literally dozens!) of Controller specs. The current attitudes are to move away from Controller specs in favor of Request specs. We have some Request specs started for our API.

See here for some more discussion: rspec/rspec-rails#1838

This is also an opportunity to review our specs and ensure:

• we have good coverage for our controller actions
• we are not testing things unnecessarily

Reference

• https://relishapp.com/rspec/rspec-rails/docs/request-specs/request-spec
• https://everydayrails.com/2016/08/29/replace-rspec-controller-tests.html

[tomrossi7]

Hi everyone! I've started work on this issue and setup this branch. I'm new to participating in Open Source, so please don't hesitate to tell me if I need to do anything differently!

[armahillo]

Awesome, thank you @tomrossi7 !

[tomrossi7]

I've gone ahead and committed the migration of the organizations_controller_spec to the organizations_requests_spec, and would love to get feedback for tackling the others.

1. I'm not sure if I should be having the conversation here on the issue or should I create a PR before its done to have the discussion?
2. What type of assertions do we want in the integration test vs. a system test? I noticed there is already an organization system test that could eliminate the entire integration test with just a few modifications. In our organization, we have moved to only create system and unit tests with very few exceptions.
3. Do we like the naming and formatting?

Thanks for letting me a part of the good you all are doing!

[holytoastr]

This branch is awesome! Where do we stand on it? I'd like to help and add a few more request specs!

[tomrossi7]

I’m traveling a ton through August and haven’t been able to push out any more tests. Feel free to jump in!

[armahillo]

@tomrossi7 Sorry, just now saw your comments!

To answer your questions:

1. I would recommend doing a series of smaller PRs that reference, but not close, this issue. Perhaps one PR per spec you create (or maybe per chunk, if you do multiple specs in a single session).
2. I'm not totally familiar with the specifics of Request specs (perhaps @holytoastr might have some insight?) but I'm thinking you'll be primarily looking for success and failure with the requests themselves, and not caring so much about whether or not the database is changed.
3. Naming and formatting looked fine in the specs I looked at -- one thing I would recommend though is to use FactoryBot factories instead of Model.create! -- many of our models have associations and the factories ensure that the associations are built out correctly. (For example: https://github.com/rubyforgood/diaper/blob/739-Migrate-Controller-Specs-to-Request-Specs/spec/requests/adjustments_requests_spec.rb#L28 -- this could be: create(:adjustment)