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

Remove controllers references from API request specs #4001

Closed
kennyadsl opened this issue Mar 19, 2021 · 5 comments
Closed

Remove controllers references from API request specs #4001

kennyadsl opened this issue Mar 19, 2021 · 5 comments
Labels
good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution

Comments

@kennyadsl
Copy link
Member

A while ago, we converted API controller specs into requests specs, see #2052. But those specs still have references from their old "life" that we can clean a little bit with a small effort.

In fact, their filenames still have the _controller_ part, which makes no sense now. Eg. solidus/api/spec/requests/spree/api/addresses_controller_spec.rb

Also, they contain references to the previous class tested as the described class. Eg.

module Spree
  describe Api::AddressesController, type: :request do
  # ...

Let's clean them!

@kennyadsl kennyadsl added good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution Beginner-Friendly labels Mar 19, 2021
@niamtokik
Copy link

Hi,

I would like to help. I am looking for a Ruby project to improve my level in this language. Seems a good issue to start.

In fact, their filenames still have the _controller_ part, which makes no sense now. Eg. solidus/api/spec/requests/spree/api/addresses_controller_spec.rb

Those files should now be renamed like solidus/api/spec/requests/spree/api/addresses_request_spec.rb, so, replacing controller by request? Or _controller_ term should just be removed?

Also, they contain references to the previous class tested as the described class. Eg.

If I take your example, module Spree should look like that after the cleanup:

module Spree
  describe Api::AddressesRequest, type: :request do
  # ...

Other dependencies in the code should be in this case modified too, right?

@jarednorman
Copy link
Member

The code after your cleanup wouldn't work, as that constant doesn't exist.

@kennyadsl Do you have any thoughts on what the convention should be for those top-level describes in request specs?

@kennyadsl
Copy link
Member Author

@niamtokik @jarednorman I didn't have anything in mind. I think we can do some research and apply the current best practices in the Rails ecosystem.

@tejasbubane
Copy link

I stubled upon this issue while looking for some beginner-level issues. I think this is resolved by #4158 and can be closed now?

@kennyadsl
Copy link
Member Author

You are right, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Wanted, but lacking time to work on it and might be a good fit for a first contribution
Projects
None yet
Development

No branches or pull requests

4 participants