From cc2e46890e38cc34e69ddb4701dd44384b4095ad Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Fri, 22 Jan 2021 11:30:05 +0100 Subject: [PATCH] Stop using deprecated req. helpers in users specs This removes the following two deprecation warnings that we are getting by millions (the two for each controller action test): ``` DEPRECATION WARNING: You are trying to generate the URL for a named route called "main_app" but no such route was found. In the future, this will result in an `ActionController::UrlGenerationError` exception. (called from process_action_with_route at /usr/ src/app/spec/support/controller_requests_helper.rb:49) DEPRECATION WARNING: Passing the `use_route` option in functional tests are deprecated. Support for this option in the `process` method (and the related `get`, `head`, `post`, `patch`, `put` and `delete` helpers) will be removed in the next version without replacement. Functional tests are essentially unit tests for controllers and they should not require knowledge to how the application's routes are configured. Instead, you should explicitly pass the appropiate params to the `process` method. Previously th e engines guide also contained an incorrect example that recommended using this option to test an engine's controllers within the dummy application. That recommendation was incorrect and has since been corrected. Instead, you should override the `@routes` variable in the test case with `Foo::Engine.routes`. See the updated engines guide for details. (called from process_action_with_route at /usr/src/app/spec/support/controller_requests_helper.rb:49) ``` It slows down our test suite and clutters the output a lot. As per my investigation, this is something that arose in https://github.com/rails/rails/pull/17453 and addressed in https://github.com/rails/rails/pull/17725. TL;DR: Engines need to define their routes in controller tests as shown in https://github.com/discourse/discourse/pull/3011. This, however, revealed a much complex reality in our case. We're still using a `Spree::Core::Engine` with its own routes at `Spree::Core::Engine.routes`. So we can't skip defining `routes { }` for each of its controllers unless we merge this engine into our app, but that's going to require more effort. What could that entail in https://github.com/openfoodfoundation/openfoodnetwork/compare/master...coopdevs:move-users-to-app-routes. To make it even worse, note that we override spree's core routes from our own, resulting in a controller whose actions are being served from routes defined in either `config/routes.rb` or `config/spree/routes.rb` :see_no_evil:. --- spec/controllers/spree/users_controller_spec.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/controllers/spree/users_controller_spec.rb b/spec/controllers/spree/users_controller_spec.rb index 8c5c6b80b1..0fa1cd4231 100644 --- a/spec/controllers/spree/users_controller_spec.rb +++ b/spec/controllers/spree/users_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Spree::UsersController, type: :controller do + routes { Spree::Core::Engine.routes } + include AuthenticationHelper describe "show" do @@ -24,7 +26,7 @@ describe Spree::UsersController, type: :controller do end it "returns orders placed by the user at normal shops" do - spree_get :show + get :show expect(orders).to include d1o1, d1o2 expect(orders).to_not include d1_order_for_u2, d1o3, d2o1 @@ -43,15 +45,17 @@ describe Spree::UsersController, type: :controller do end describe "registered_email" do + routes { Openfoodnetwork::Application.routes } + let!(:user) { create(:user) } it "returns true if email corresponds to a registered user" do - spree_post :registered_email, email: user.email + post :registered_email, email: user.email expect(json_response['registered']).to eq true end it "returns false if email does not correspond to a registered user" do - spree_post :registered_email, email: 'nonregistereduser@example.com' + post :registered_email, email: 'nonregistereduser@example.com' expect(json_response['registered']).to eq false end end @@ -59,14 +63,14 @@ describe Spree::UsersController, type: :controller do context '#load_object' do it 'should redirect to signup path if user is not found' do allow(controller).to receive_messages(spree_current_user: nil) - spree_put :update, user: { email: 'foobar@example.com' } + put :update, user: { email: 'foobar@example.com' } expect(response).to redirect_to('/login') end end context '#create' do it 'should create a new user' do - spree_post :create, user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' } + post :create, user: { email: 'foobar@example.com', password: 'foobar123', password_confirmation: 'foobar123' } expect(assigns[:user].new_record?).to be_falsey end end